RFR:8208531 : -javafx mode should be on by default when JavaFX is available

Priya Lakshmi Muthuswamy priya.lakshmi.muthuswamy at oracle.com
Thu Sep 27 08:57:43 UTC 2018


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



More information about the javadoc-dev mailing list