[verona.stage] RFR 8087203: Add support for PATCH field and remove unused fields of new version string
Alejandro E Murillo
alejandro.murillo at oracle.com
Thu Jun 18 22:56:11 UTC 2015
On 6/18/2015 7:41 AM, Alan Bateman wrote:
> 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.
in general, that code can be called a lot, so changes need to be
as to avoid perf impact. So it might be better if that check can be removed
> 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.
yeah, I had meant to double check that. Will change it to 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.
sure, will make it into several lines, based on components, like this:
String jep223Pattern =
"^([0-9]+)(\\.([0-9]+))?(\\.([0-9]+))?(\\.([0-9]+))?" + // $VNUM
"(-([a-zA-Z]+))?(\\.([a-zA-Z]+))?" + // $PRE
"(\\+([0-9]+))?" + // Build Number
"(([-a-zA-Z0-9.]+))?$"; // $OPT
see new webrev:
> Otherwise looks okay to me.
Here's the new webrev (I tested these with JPRT, testsets hotspot and pit):
More information about the hotspot-dev