Katja, The policy for reviews in the jdk repo is different from the hotspot repo. For the jdk it is ok with a single review.
/Staffan On 16 jun 2014, at 09:59, Yekaterina Kantserova <[email protected]> wrote: > Hi, > > could I get one more review please? > > Thanks, > Katja > > > On 06/13/2014 02:23 PM, Staffan Larsen wrote: >> Looks good! >> >> Thanks, >> /Staffan >> >> On 13 jun 2014, at 14:15, Yekaterina Kantserova >> <[email protected]> wrote: >> >>> Staffan, thanks for your review! >>> >>> The new webrev can be found here: >>> http://cr.openjdk.java.net/~ykantser/6545295/webrev.01/ >>> >>> // Katja >>> >>> On 06/13/2014 01:44 PM, Staffan Larsen wrote: >>>> Katja, >>>> >>>> Great to see this simplification! A couple of comments: >>>> >>>> - I don’t think you need to launch jhat with -XX:+UsePerfData - no one is >>>> attaching or reading data from the process. >>>> >>>> - The static processBuilder field seems to be mis-used. Why not use >>>> different ProcessBuilder objects for the HelloWorld and jhat? >>>> >>>> - For dumpFile.deleteOnExit() to be useful you should add it as early as >>>> possible (line 48 perhaps?). If it is the last line in the test you can >>>> just as well do dumpFile.delete(). >>> >>> Of cause it should be dumpFile.delete()! - dumpFile.deleteOnExit() has >>> crept in by mistake. >>> >>>> >>>> /S >>>> >>>> >>>> On 13 jun 2014, at 13:25, Yekaterina Kantserova >>>> <[email protected]> wrote: >>>> >>>>> Hi, >>>>> >>>>> Could I please have a review of this fix. >>>>> >>>>> webrev: http://cr.openjdk.java.net/~ykantser/6545295 >>>>> bug: https://bugs.openjdk.java.net/browse/JDK-6545295 >>>>> >>>>> The test has been re-written using the testlibrary's functionality. It's >>>>> verified internally on all basic platforms. >>>>> >>>>> Thanks, >>>>> Katja >> >
