Jaroslav,
> * instead of
> `Arrays.toString(processBuilder.command().toArray()).replace(",", "")`
> you can use
> `processBuilder.command().stream().collect(Collectors.joining(" "))` -
> which is slightly more concise
> * typo 'depends *to* operating system' -> 'depends *on* operating system'fixed. (in-place, press shift-reload). -Dmitry On 2015-08-21 14:44, Jaroslav Bachorik wrote: > On 21.8.2015 12:05, Dmitry Samersoff wrote: >> On 2015-08-21 12:35, Jaroslav Bachorik wrote: >>> Hi Dmitry, >>> >>> On 19.8.2015 19:10, Dmitry Samersoff wrote: >>>> Everybody, >>>> >>>> Please, review test-only fix. >>>> >>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8086134/webrev.01/ >>>> >>>> The test is rewritten to use testlibrary and to detect common bad >>>> environment conditions. >>>> >>>> Tests in Hotspot and JDK workspaces are identical, except imports and >>>> the fact that JDK version runs >>>> jstack but hotspot version runs jhsdb (sa launcher). >>> >>> I will need more explanation here. >>> >>> * the issue is about the tmtools test failing; are you planning to >>> remove the failing tests? >> >> Yes. Falling test have to be removed. >> >>> * what is the reason for having the test in hotspot and in jdk? >>> wouldn't it be possible to have it just in jdk and launch it once >>> using jstack and once jhsdb? >> >> Currently, tests for tmtools (jstack, jmap etc) resides in JDK, but >> tests for SA resides in Hotspot. I would love to place all tests to the >> single place >> but this work is out of scope of this fix. > > Ok. Thanks. > > Overall looks good. Just some minor nits. > > * instead of > `Arrays.toString(processBuilder.command().toArray()).replace(",", "")` > you can use > `processBuilder.command().stream().collect(Collectors.joining(" "))` - > which is slightly more concise > * typo 'depends *to* operating system' -> 'depends *on* operating system' > > -JB- > > >> >> -Dmitry >> >>> >>> >>> Thanks, >>> >>> -JB- >>> >>>> >>>> -Dmitry >>>> >>>> >>> >> >> > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
