Hi Jaroslav, Thanks for the comments. I think they are valid, but I would prefer to do them in a separate changeset if that is ok with you?
I still need a JDK 8u Reviewer to look at this. /Staffan On 4 sep 2014, at 18:56, Jaroslav Bachorik <[email protected]> wrote: > Hi Staffan, > > > On 09/03/2014 02:27 PM, Staffan Larsen wrote: >> …aaaand the link: >> http://cr.openjdk.java.net/~sla/8044398-8039173-8044135-jdk8u/webrev.00/ > > I have just nits, probably not important because this is a backport and it > should not differ from the original patch if not really necessary. > > jdk/src/share/classes/sun/tools/attach/HotSpotVirtualMachine.java > * readErrorMessage() - you might want to use `BufferedReader br = new > BufferedReader(new InputStreamReader(is))` and `String line = br.readLine()` > to read the text contents > > * L310 Use StringBuilder instead > > jdk/test/sun/management/jmxremote/startstop/JMXStartStopTest.java > * L76 - you can use the concise version `for (ObjectName name : names) {...}` > of for loop and forget the iterator > > The rest is fine. Thumbs up! > > -JB- > >> >> /Staffan >> >> On 3 sep 2014, at 14:25, Staffan Larsen <[email protected]> wrote: >> >>> This is a review for a backport of JDK-8044135 to jdk8u. This fix had a few >>> dependencies that also needed backporting, so included in this review are >>> the following fixes from JDK9: >>> >>> • 8044135 Add API to start JMX agent from attach framework (jdk) >>> • 8039173 Propagate errors from Diagnostic Commands as exceptions in the >>> attach framework (jdk) >>> • 8044398 Attach code should propagate errors in Diagnostic Commands as >>> errors (hotspot) >>> >>> The only significant change from the version in JDK 9 is that >>> startManagementAgent() and startLocalManagementAgent() in >>> VirtualMachine.java have "@since 1.8" (instead of 1.9). >>> >>> After speaking with the maintainers and gatekeepers, I plan to push this to >>> the jdk8u-hs-dev repository. >>> >>> Thanks, >>> /Staffan >>> >> >
