Robbin, The fix looks good for me.
-Dmitry On 2016-06-02 09:41, Robbin Ehn wrote: > Hi Dmitry, thanks for looking at this. > > On 06/01/2016 07:59 PM, Dmitry Samersoff wrote: >> Robbin, >> >> ProcessArgumentMatcher.java:61 >> >> What is the reason of keeping singlePid as a string. It might be >> better to convert it to Long at the argument processing time and then >> deal with Long. > > VirtualMachineDescriptor returns pid as string, see > ProcessArgumentMatcher.java:116 > So I'll prefer to keep it as a string. > > Rest should be fixed, also contains fixes after input from Staffan > (thanks). > > Inc: http://cr.openjdk.java.net/~rehn/8156537/02-01/webrev/ > Full: http://cr.openjdk.java.net/~rehn/8156537/02/webrev/ > > /Robbin > >> >> Arguments.java: >> >> null seems more natural to me than "0" >> >> JInfo.java: >> >> 54 value of flag is not obvious to me, because optionCount do exactly >> the same. >> >> Update: it's the open question how we should proceed multiple -flag >> and -flags arguments. >> >> e.g. command line >> >> jinfo -flags -flag MinHeapFreeRatio=1 pidA pidA >> >> results different output for the same process. >> >> I didn't find any documentation specified it, So I would prefer to >> disable the combination like one above. At least for now. >> >> 74 missed space >> >> MonitoredVmUtil.java: >> >> 119 Could you change tmp to something more descriptive? Or ever >> better move the code guessing lastFileSparator to a separate function >> for better readability. >> >> -Dmitry >> >> On 2016-06-01 10:16, Robbin Ehn wrote: >>> Hi all, >>> >>> There were some ripple effects, so to get everything working as intended >>> the patch grow slightly. >>> >>> Webrev: http://cr.openjdk.java.net/~rehn/8156537/01/webrev/ >>> Bug: https://bugs.openjdk.java.net/browse/JDK-8156537 >>> >>> Both a lot of manual testing, the new tests and >>> jdk/test/sun/tools/,jdk/test/tools/ and some other which uses e.g. jcmd. >>> >>> Thanks! >>> >>> /Robbin >>
