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. 

Regards,
Rajeev Chamyal

-----Original Message-----
From: Sergey Bylokhov 
Sent: 19 October 2015 02:29
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@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@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.

Reply via email to