RFR: 8072379: Implement jdk.Version and jdk.OracleVersion

Iris Clark iris.clark at oracle.com
Fri Dec 18 21:56:36 UTC 2015


Hi, Magnus.

Thanks for the review comments.

>>      http://cr.openjdk.java.net/~iris/verona/8072379/webrev.1/

> I thought the end agreement was that the + should always be present even if build was empty, if opt was present but not pre. That is, "9-foo" 
> should unambigiously parse as vnum=9 and pre="foo", while "9-+foo" 
> should umambigously parse as vnum=9 and opt="foo". The javadoc does not seem to reflect this.

You have correctly summarized the expectation and the implementation.  I've made the following update the class javadoc:

From:

   * <blockquote><pre>
   *     $VNUM(-$PRE)?(\+$BUILD)?(-$OPT)?
   * </pre></blockquote>

To:

   * <blockquote><pre>
   *     $VNUM(-$PRE)?(\+($BUILD)?(-$OPT)?)?
   * </pre></blockquote>

> The comparison of two version strings which differs only in "pre" does not adhere to the principle of astonishment.
> 
> The documentation states: "Pre-release identifiers are compared numerically when they consist only of digits, and lexiographically otherwise.  Numeric identifiers are considered to be less than non-numeric identifiers." But consider the following version strings:
> 
> 9-01
> 9-01a
> 9-02
> 9-003b
>
> That sequence would be ordered like this, which I find highly surprising.
> 9-01
> 9-02
> 9-003b
> 9-01a

The current choice of comparison is motivated by Semantic Versioning.  A goal of jdk.Version is to subset this scheme.  The sorting may be counter-intuitive and it is possible that in the future we may decide that perfect alignment with semver.org is not critical.  For now, we're settling with documenting the behavior.

> I'm also surprised that equals() does not produce the same result as compareTo(). If opt is to be ignored, surely it should be so in equals() as well?

Based on your comment and Joe's, I've added comparison methods compareTo()/equals() and compareToIgnoreOpt()/equalsIngnoreOpt() for consistency.

I'll be sending an updated webrev shortly.

Thanks,
iris



More information about the core-libs-dev mailing list