[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR: 8039173 Propagate errors from Diagnostic Commands as exceptions in the attach framework
From: Staffan Larsen <staffan.larsen () oracle ! com>
Date: 2014-04-29 14:08:50
Message-ID: 71C097AB-E539-45C3-AD05-5E046751B7A9 () oracle ! com
[Download RAW message or body]
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.Bateman@oracle.com> wrote:
> On 16/04/2014 10:21, Staffan Larsen wrote:
> > On 14 apr 2014, at 17:17, Alan Bateman <Alan.Bateman@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.
>
>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic