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

[huizhe wang]

Hi Alejandro,

Alan's right about the JAXP version-check code. But since this is 
urgently needed, I agree you can push the change as is. I'll create a 
separate bug to re-examine version related code in jaxp. As Alan pointed 
out, it would be some clean-up.

Thanks,
Joe

On 6/19/2015 9:53 AM, Alejandro E Murillo wrote:
>
> Hi Alan, just to you.
> didn't hear back from you, so I'll assume you are fine with
> the corrections. We want to get this into further testing
> so I'm going to push the changes
>
> cheers
> Alejandro
>
> On 6/18/2015 4:56 PM, Alejandro E Murillo wrote:
>>
>> 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!
>>
>