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 >
