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.
> 
> 

Reply via email to