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

[Jonathan Gibbons]

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
>