<Swing Dev> JDK9 Review Request for 8138881: Bug in OSInfo.java

[Rajeev Chamyal]

Hello Sergey,

I have updated the test below is the updated webrev.
Webrev : http://cr.openjdk.java.net/~rchamyal/8138881/webrev.02/

Regards,
Rajeev Chamyal

-----Original Message-----
From: Sergey Bylokhov 
Sent: 20 October 2015 04:30
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev at openjdk.java.net
Subject: Re: <Swing Dev> JDK9 Review Request for 8138881: Bug in OSInfo.java

On 19.10.15 0:59, Rajeev Chamyal wrote:
> Hello Sergey,
>
> Thanks for the review. The test was working before the fix as well.
> The current implementation of OSInfo.getWindowsVersion() adds the missing values also to windowsVersionMap.
> So test always passes.

Then the test should be reworked, is my assumption correct that before this fix the method OSInfo.getWindowsVersion() will return WINDOWS_7 on windows vista?

>
> Regards,
> Rajeev Chamyal
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: 19 October 2015 02:29
> To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev at openjdk.java.net
> Subject: Re: <Swing Dev> JDK9 Review Request for 8138881: Bug in OSInfo.java
>
> After some additional review I am not sure that this test is useful, are you sure that the test fails before the fix?
>
> On 15.10.15 11:26, Rajeev Chamyal wrote:
>> Hello Sergey,
>>
>> Thanks for the review. I have updated the webrev with review comments.
>>
>> WebRev : http://cr.openjdk.java.net/~rchamyal/8138881/webrev.01/
>>
>> Regards,
>> Rajeev Chamyal
>>
>> -----Original Message-----
>> From: Sergey Bylokhov
>> Sent: 14 October 2015 20:44
>> To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev at openjdk.java.net
>> Subject: Re: <Swing Dev> JDK9 Review Request for 8138881: Bug in
>> OSInfo.java
>>
>> Hi, Rajeev.
>> The fix looks fine, a few comments about the test:
>>     - Please use some useful name instead of bugNumber.
>>     - @test @bug should be in the different rows.
>>     - This test uses classes from the "sun.awt." package, which means that soon by default it will not be available to the test. You need to add:
>>       * @modules java.desktop/sun.awt
>>          You can check it on the latest jigsaw builds:
>>          https://jdk9.java.net/jigsaw/
>>
>> On 14.10.15 15:51, Rajeev Chamyal wrote:
>>> Hello,
>>>
>>> Please review the following fix for Jdk9:
>>>
>>> WebRev : http://cr.openjdk.java.net/~rchamyal/8138881/webrev.00/
>>>
>>> Bug Id: https://bugs.openjdk.java.net/browse/JDK-8138881
>>>
>>> Issue: There was a typo in OSInfo.java.
>>>
>>> Regards,
>>>
>>> Rajeev Chamyal
>>>
>>
>>
>> --
>> Best regards, Sergey.
>>
>
>
> --
> Best regards, Sergey.
>


-- 
Best regards, Sergey.