On 16/06/2015 23:55, Alejandro E Murillo wrote:
Please review these changes: Bug: https://bugs.openjdk.java.net/browse/JDK-8087202 Webrev: http://cr.openjdk.java.net/~amurillo/9/8087202
The implementation of isJavaVersionAtLeast in the JAXP classes look okay although I think this is code that could be removed. Joe Wang can confirm but I think it dates back to when the JAXP API was a standalone API and there was an attempt to keep the code in sync across major versions. I'll create a separate bug re-examine this as it looks like some clean-up can be done here.
Just on the comment "In JDK9 the version string was changed ..." will date quickly and would be nice to say that "In JDK 8 and older then it assumes 1.N and for JDK 9 and newer it assumes N.
In sun.misc.Version.initVersions then InternalError instead of RuntimeException might be more appropriate as something is really broken if that happens.
Also as David pointed out, it shouldn't be @since JDK9 in the new method. This reminds me to check if the JEP says anything about @since tags because we already have quite a few @since 1.9.
In the Version.java test then the line with the pattern is really long, maybe that could be split up to make future side-by-side views easier to read.
Otherwise looks okay to me. -Alan.
