Looks good.
-- Jon
On 09/27/2018 01:57 AM, Priya Lakshmi Muthuswamy wrote:
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
