Hi Jon,

Thanks for the review.
Updated webrev : http://cr.openjdk.java.net/~pmuthuswamy/8208531/webrev.01/
I have verified the testcase with JavaFX.

Thanks,
Priya


On 9/27/2018 3:39 AM, Jonathan Gibbons wrote:


On 09/25/2018 05:22 AM, Priya Lakshmi Muthuswamy wrote:
Hi,

Kindly review the fix for https://bugs.openjdk.java.net/browse/JDK-8208531
webrev : http://cr.openjdk.java.net/~pmuthuswamy/8208531/webrev.00/
CSR: https://bugs.openjdk.java.net/browse/JDK-8208532/

Thanks,
Priya

BaseConfiguration
Suggest blank line after 403;
suggest { } around the statement after if.

getQualifiedName().toString().equals

can be simplified to getQualifiedName().contentEquals to avoid the intermediate String.

Overall, it might be simpler to write

1361             return javafxModule == null || javafxModule.isUnnamed()
                        || javafxModule.getQualifiedName().contentEquals("javafx.base")) {



Test file

Wrong copyright on test file.  Tests should not use the copyright containing
the classpath exception.

Suggest blank line before line 47

In the sanity() method, you can delete 49, and change 51 to Class.forName("javafx.beans.Observable")

Suggest moving main to before instance methods, instead of amongst the
instance methods.

Please confirm that you have run the test with JavaFX available, to verify that it is not passing by default.  A slightly better approach would be to
mock up empty classes for whatever you need and to put those classes on
the classpath when you run javadoc.

-- Jon

Reply via email to