Looks good. Reviewed-by: emcmanus
Éamonn 2012/10/25 Jaroslav Bachorik <[email protected]> > 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 <[email protected]> > > > >> 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- > >>> > >> > >> > > > >
