Looks good to me, too. /Magnus
> 4 dec. 2018 kl. 20:34 skrev Mandy Chung <mandy.ch...@oracle.com>: > > The revised webrev looks okay. > > Mandy > >> On 12/4/18 11:32 AM, Roger Riggs wrote: >> Hi Mandy, Martin, >> >> The new test is unnecessary, the case is covered by >> java/lang/System/Versions test >> and uses the stronger comparison for the version numbers. >> >> It would not detect the problem unless the version included more than the >> major version. >> >> Webrev: http://cr.openjdk.java.net/~rriggs/webrev-spec-ver-8214700-3/ >> >> Thanks, Roger >> >>> On 12/04/2018 01:41 PM, Mandy Chung wrote: >>> >>> >>>> On 12/4/18 8:16 AM, Roger Riggs wrote: >>>> Please review correctly setting the java.specification.version property >>>> with only the major version number. A test is added to ensure the >>>> java spec version agrees with the major version. >>>> >>>> The symptoms are that jtreg would fail with a full version number. >>>> >>>> Webrev: >>>> http://cr.openjdk.java.net/~rriggs/webrev-spec-ver-8214700-2/ >>>> >>> >>> Looks okay. I agree with Martin to go with a stronger assertion without >>> converting into a number. test/jdk/java/lang/System/Versions.java looks >>> like also covering this test case. At some point it'd be good to >>> consolidate these two tests. >>> >>> Nit: in GensrcMisc.gmk, I think VERSION_NUMBER and VERSION_PRE etc are a >>> relevant group. VERSION_SPECIFICATION can be moved to group with >>> VERSION_CLASSFILE_MAJOR and MINOR. Magnus may have an opinion. >>> >>> Mandy >> >
