Hi Igor,

Removing trailing spaces is fine.

Thanks, Roger


On 6/7/2017 11:29 AM, Igor Ignatyev wrote:
Hi Roger,

please see my comments inline.

-- Igor

On Jun 7, 2017, at 7:48 AM, Roger Riggs <roger.ri...@oracle.com> wrote:

Hi Igor,

It is odd that test/TEST.ROOT shows up in the webrev with no changes.
it seems there were trailing spaces at line#13[1] which my vi removed on 
closing. if you want, I can revert that before pushing.

[1]
-# run. Tests that are not headful are "headless."
+# run. Tests that are not headful are "headless."


Seeing @library with both /lib/testlibrary and /test/lib makes me wonder when 
that will get resolved?
this will be resolved that the rest of testlibrary from /lib/testlibrary will 
be merged w/ library in /test/lib by JDK-8075327[2]. I expect at least 4 other 
sub-tasks.

[2] https://bugs.openjdk.java.net/browse/JDK-8075327
Otherwise, looks fine
thank you for your review.

Thanks, Roger

On 6/7/2017 2:44 AM, Igor Ignatyev wrote:
still looking for a Reviewer.

-- Igor
On May 31, 2017, at 2:57 PM, Igor Ignatyev <igor.ignat...@oracle.com> wrote:

Hi Mandy,

8180793[1], which moved j.t.l.wrappers.* to j.t.lib package, has been 
propagated to jdk10/jdk10. I have updated the patch accordingly -- 
http://cr.openjdk.java.net/~iignatyev//8180386/webrev.01/index.html
could you please take a look?

[1] https://bugs.openjdk.java.net/browse/JDK-8180793

Thanks,
-- Igor

On May 18, 2017, at 2:49 PM, Igor Ignatyev <igor.ignat...@oracle.com> wrote:

On May 18, 2017, at 2:41 PM, Mandy Chung <mandy.ch...@oracle.com> wrote:


On May 18, 2017, at 2:37 PM, Igor Ignatyev <igor.ignat...@oracle.com> wrote:


On May 18, 2017, at 2:34 PM, Mandy Chung <mandy.ch...@oracle.com> wrote:
This comment is not related to the change.  The package name 
`jdk.test.lib.wrappers` is odd. TimeLimitedRunner and InfiniteLoop classes 
could simply be in the jdk.test.lib package or jdk.test.lib.util package.

Mandy

agree, I'll file an RFE to find a better package for TimeLimitedRunner and 
InfiniteLoop classes.
Why not doing the rename with this patch?
there are hotspot tests which use jdk.test.lib.wrappers.TimeLimitedRunner and 
InfiniteLoop, so it will require changes in hotspot repo. I'd prefer to have 
removing classes from jdk testlibrary and renaming classes in top level 
testlibrary as separate patches for clarity purposes.

-- Igor

Reply via email to