Looks good.

Reviewed-by: emcmanus

Éamonn


2012/10/25 Jaroslav Bachorik <jaroslav.bacho...@oracle.com>

> Thanks, addressing the comments ...
>
> On 10/24/2012 05:43 PM, Eamonn McManus wrote:
> > The fix looks good, but I would adjust a couple of small things. First,
> you
> > can improve the existing and new code with multi-catch. Second, the
> message
> > in the if (notFoundCount > 0) block should be adjusted to take into
> account
> I was not really sure about the wording of the message - I've added the
> part warning about incompatibility. But if you have something better in
> mind I would happily use it.
>
> > the new cases. Finally, hardwiring port 12345 into the test makes it a
> bit
> > fragile. It would be better for the server to listen on any available
> port
> > and (for example) write which port it is, or what its JMXServiceURL is,
> on
> > its stdout. The shell script can then read that and launch the client
> with
> > it. That would also get rid of the arbitrary sleep 3.
> Done and done.
>
> Webrev at http://cr.openjdk.java.net/~jbachorik/JDK-6937053/webrev.02/
>
> -JB-
>
> >
> > Éamonn
> >
> >
> > 2012/10/24 Jaroslav Bachorik <jaroslav.bacho...@oracle.com>
> >
> >> Updated webrev at
> >> http://cr.openjdk.java.net/~jbachorik/JDK-6937053/webrev.01/ - removed
> a
> >> dangling debug output.
> >>
> >> -JB-
> >>
> >> On 10/24/2012 04:03 PM, Jaroslav Bachorik wrote:
> >>> I am looking for review and a sponsor.
> >>>
> >>> Webrev available at
> >>> http://cr.openjdk.java.net/~jbachorik/JDK-6937053/webrev.00/
> >>>
> >>> The RMI marshalling process may throw java.rmi.UnmarshallException eg.
> >>> in cases of incompatible changes in enums. The bad thing is that
> >>> ClientNotifForwarder chooses to silently die instead of reporting the
> >>> problem.
> >>>
> >>> The fix consists of adding support for handling
> >>> java.rmi.UnmarshallException the same way as
> >>> java.io.NotSerializableException and appropriate changes in the
> javadoc.
> >>>
> >>> Thanks,
> >>>
> >>> -JB-
> >>>
> >>
> >>
> >
>
>

Reply via email to