Re: [9] Review request for 8033699: Incorrect radio button behavior

2014-09-15 Thread Vivi An
Fixed. New Webrev: http://cr.openjdk.java.net/~van/8033699/webrev.03/ Thanks Alex. Regards, ~ Vivi On 9/15/2014 2:59 AM, Alexander Scherbatiy wrote: On 9/12/2014 8:56 PM, Vivi An wrote: Thanks Alexander. All items addressed except last one, please see comments inline. New Webrev: http://cr.

Re: [9] Review Request: 8055326 Fix typos in client-related packages

2014-09-15 Thread Phil Race
approved. -phil. On 9/10/2014 5:11 AM, Sergey Bylokhov wrote: Hi, Phil. It seems both changes are unnecessary: http://cr.openjdk.java.net/~serb/8055326/webrev.01 On 21.08.2014 22:04, Phil Race wrote: Was the additional spce on the 2nd line intended here ? --- old/src/java.desktop/share/clas

Re: [9] Review Request: 8055326 Fix typos in client-related packages

2014-09-15 Thread Alexander Zvegintsev
Hello Sergey, The fix looks good to me in general, but I have a few comments: It would be nice to add @Override annotation to java.awt.Window.adjustDescendantsOnParent() --- old/src/java.desktop/share/classes/java/awt/Component.java 2014-09-10 15:11:22.601342000 +0400 +++ new/src/java.deskt

Re: [9] Review Request: 8055326 Fix typos in client-related packages

2014-09-15 Thread Alexander Scherbatiy
The fix looks good to me. Thanks, Alexandr. On 9/10/2014 4:11 PM, Sergey Bylokhov wrote: Hi, Phil. It seems both changes are unnecessary: http://cr.openjdk.java.net/~serb/8055326/webrev.01 On 21.08.2014 22:04, Phil Race wrote: Was the additional spce on the 2nd line intended here ? --

Re: [9] Review Request for 8056991: Provide OSInfo functionality to regression tests

2014-09-15 Thread Yuri Nesterenko
Thank you Alexander! I hope I may push it with your single review: the change is in test area and seems virtually harmless. Cheers, -yan On 09/15/2014 03:13 PM, Alexander Scherbatiy wrote: The fix looks good to me. Thanks, Alexandr. On 9/15/2014 3:00 PM, Yuri Nesterenko wrote: On 09

Re: [9] Review Request for 8056991: Provide OSInfo functionality to regression tests

2014-09-15 Thread Alexander Scherbatiy
The fix looks good to me. Thanks, Alexandr. On 9/15/2014 3:00 PM, Yuri Nesterenko wrote: On 09/15/2014 02:42 PM, Alexander Scherbatiy wrote: test/lib/testlibrary/jdk/testlibrary/OSInfo.java 32 import static jdk.testlibrary.OSInfo.OSType.*; Is this import necessary? Yes: many tests use

Re: [9] Review Request for 8056991: Provide OSInfo functionality to regression tests

2014-09-15 Thread Yuri Nesterenko
On 09/15/2014 02:42 PM, Alexander Scherbatiy wrote: test/lib/testlibrary/jdk/testlibrary/OSInfo.java 32 import static jdk.testlibrary.OSInfo.OSType.*; Is this import necessary? Yes: many tests use enum without qualification -- it's short and presumably elegant. Without this import usage like O

Re: [9] Review Request for 8056991: Provide OSInfo functionality to regression tests

2014-09-15 Thread Alexander Scherbatiy
test/lib/testlibrary/jdk/testlibrary/OSInfo.java 32 import static jdk.testlibrary.OSInfo.OSType.*; Is this import necessary? 115 public static PrivilegedAction getOSTypeAction() { 116 return osTypeAction; 117 } Is this method used in the tests? Thanks, Alexandr. On 9/15/20

Re: [9] Review request for 8033699: Incorrect radio button behavior

2014-09-15 Thread Alexander Scherbatiy
On 9/12/2014 8:56 PM, Vivi An wrote: Thanks Alexander. All items addressed except last one, please see comments inline. New Webrev: http://cr.openjdk.java.net/~van/8033699/webrev.02/ 163 if ( keyListener != null ) { Could you remove spaces in the brackets? After code formattin

[9] Review Request for 8056991: Provide OSInfo functionality to regression tests

2014-09-15 Thread Yuri Nesterenko
Dear friends, one more weekly reminder! Without this (or similar) fix applied, we cannot start changes of ~60 regtests, and time is short. Cheers, -yan On 09/08/2014 01:09 PM, Yuri Nesterenko wrote: Weekly reminder! Cheers, -yan On 09/01/2014 12:44 PM, Yuri Nesterenko wrote: Colleagues, ple