The fix looks good to me.
Thanks,
Alexandr.
On 10/20/2015 4:41 PM, Sergey Bylokhov wrote:
The fix looks fine.
On 20.10.15 14:35, Rajeev Chamyal wrote:
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; [email protected]
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; [email protected]
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; [email protected]
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.
