On 8/18/2011 1:14 PM, Alan Bateman wrote:
Sebastian Sickelmann wrote:
Hi,
i have created a fix for fixing Exception-Chains in case of an
rethrown RuntimeException.
I am not quite sure if this is inside the scope of what i
discussed[0][1] with Joe. But it is
fixed in the same manner as the patches there.
http://oss-patches.24.eu/openjdk8/RuntimeException/REBASED_ON_07ad16388170/
Someone who wants to review / sponsor this?
I agree with Andrew that this is good clean-up.
I think the changes to
src/share/classes/javax/xml/crypto/NoSuchMechanismException.java need
closer examination. Removing the 3 methods that it overloads should be
okay. Adding @since 1.8 to the existing constructor is not okay
Removing the cause field given that it's serializable, I need to
remind myself how this works.
A minor nit but I notice in many places where you've introduced
multicatch that you put the vertical bar at the beginning of the next
line whereas I think we've mostly been putting it on the right. In
src/share/classes/javax/management/relation/RelationService.java I see
you've got both. I don't know if coding conventions have been
established on this.
For multi-catch, I would actually recommend having the multiple
exceptions listed on a single line, subject go general line length concerns.
Personally I find
catch(FirstException | SecondException e) {...}
to be more readable than
catch(FirstException
| Second Exception) {...}
I agree with Alan that if the sequence of exception types is going to be
split across lines, it is better to be split as
catch(First Exception |
SecondException e)
rather than having the "|" start the subsequent line.
Cheers,
-Joe
