[verona.stage] RFR 8087203: Add support for PATCH field and remove unused fields of new version string

[Alejandro E Murillo]

Thanks Alan,
see below

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.
sounds good,
in   general, that code can be called a lot, so changes need to be 
carefully done
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.
Indeed. Changed.
>
> 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:
http://cr.openjdk.java.net/~amurillo/9/8087202.v2/jdk/src/java.base/share/classes/sun/misc/Version.java.template.udiff.html

>
> Otherwise looks okay to me.
great, thanks!

Here's the new webrev (I tested these with JPRT, testsets hotspot and pit):

http://cr.openjdk.java.net/~amurillo/9/8087202.v2/

Thanks!

-- 
Alejandro