Thanks for the comments. I have updated the review with both. webrev: http://cr.openjdk.java.net/~sla/8039173/webrev.05/
I do not actually have any Reviewers for this change. Anyone willing to take it on is much appreciated. Thanks, /Staffan On 29 apr 2014, at 15:22, Alan Bateman <alan.bate...@oracle.com> wrote: > On 16/04/2014 10:21, Staffan Larsen wrote: >> On 14 apr 2014, at 17:17, Alan Bateman <alan.bate...@oracle.com> wrote: >> >>> : >>> For someone looking at the Virtualmachine API then I don't think the >>> javadoc is clear enough to understand when one might get the specific >>> AttachOperationFailedException vs. the more general IOException. I think it >>> means that there was communication with the target VM but that the >>> operation failed for some reason but I don't think this will be obvious to >>> the reader. >> I have tried to clarify the wording in the javadoc. Suggestions for >> improvements are welcome. > Sorry for the delay, I was on away for a few days and just catching up with > this again. > > The updated descriptions looks much better. For IOException then it might be > better to have a bit of wriggle room to allow for other I/O errors that might > not be communication related. So maybe something like "If an I/O error > occurs, a communication error for example, that cannot be identified as an > error to indicate that the operation failed in the target VM". > >> : >>> For the new exception then it would be good to add @since and also a >>> copyright header. >> Fixed. >> > Thanks, a formatting nit at L42, I assume that the "{" will fit at the end of > line 41. > > I don't have cycles at the moment to go through the implementation changes > but I think you have other reviewers for that. > > -Alan. > >