[ https://issues.apache.org/jira/browse/MNG-7559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17633329#comment-17633329 ]
ASF GitHub Bot commented on MNG-7559: ------------------------------------- elharo commented on code in PR #845: URL: https://github.com/apache/maven/pull/845#discussion_r1020904523 ########## maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java: ########## @@ -41,19 +42,34 @@ * <code>1.0alpha1 => [1, 0, alpha, 1]</code></li> * <li>unlimited number of version components,</li> * <li>version components in the text can be digits or strings,</li> - * <li>strings are checked for well-known qualifiers and the qualifier ordering is used for version ordering. - * Well-known qualifiers (case insensitive) are:<ul> - * <li><code>alpha</code> or <code>a</code></li> - * <li><code>beta</code> or <code>b</code></li> - * <li><code>milestone</code> or <code>m</code></li> - * <li><code>rc</code> or <code>cr</code></li> - * <li><code>snapshot</code></li> - * <li><code>(the empty string)</code> or <code>ga</code> or <code>final</code></li> - * <li><code>sp</code></li> - * </ul> - * Unknown qualifiers are considered after known qualifiers, with lexical order (always case insensitive), - * </li> - * <li>a hyphen usually precedes a qualifier, and is always less important than something preceded with a dot.</li> + * <li> + * string qualifiers are ordered lexically, with the following exceptions: + * <ul> + * <li> 'snapshot' < '' < 'sp' </li> + * </ul> + * and alias => replacement: + * <ul> + * <li> 'a' => 'alpha' </li> + * <li> 'b' => 'beta' </li> + * <li> 'm' => 'milestone' </li> + * <li> 'cr' => 'rc' </li> + * <li> 'final' = 'ga' = 'release' => '' </li> + * </ul> + * </li> + * <li> + * following semver rules is encouraged, and some qualifiers are discouraged: Review Comment: Following ########## maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java: ########## @@ -41,19 +42,34 @@ * <code>1.0alpha1 => [1, 0, alpha, 1]</code></li> * <li>unlimited number of version components,</li> * <li>version components in the text can be digits or strings,</li> - * <li>strings are checked for well-known qualifiers and the qualifier ordering is used for version ordering. - * Well-known qualifiers (case insensitive) are:<ul> - * <li><code>alpha</code> or <code>a</code></li> - * <li><code>beta</code> or <code>b</code></li> - * <li><code>milestone</code> or <code>m</code></li> - * <li><code>rc</code> or <code>cr</code></li> - * <li><code>snapshot</code></li> - * <li><code>(the empty string)</code> or <code>ga</code> or <code>final</code></li> - * <li><code>sp</code></li> - * </ul> - * Unknown qualifiers are considered after known qualifiers, with lexical order (always case insensitive), - * </li> - * <li>a hyphen usually precedes a qualifier, and is always less important than something preceded with a dot.</li> + * <li> + * string qualifiers are ordered lexically, with the following exceptions: + * <ul> + * <li> 'snapshot' < '' < 'sp' </li> + * </ul> + * and alias => replacement: + * <ul> + * <li> 'a' => 'alpha' </li> + * <li> 'b' => 'beta' </li> + * <li> 'm' => 'milestone' </li> + * <li> 'cr' => 'rc' </li> + * <li> 'final' = 'ga' = 'release' => '' </li> + * </ul> + * </li> + * <li> + * following semver rules is encouraged, and some qualifiers are discouraged: Review Comment: semver --> Semantic Versioning 1.0.0 2.0 semver actually diverges from Maven's rules in a few respects, though in most common cases they're the same. ########## maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java: ########## @@ -41,19 +42,34 @@ * <code>1.0alpha1 => [1, 0, alpha, 1]</code></li> * <li>unlimited number of version components,</li> * <li>version components in the text can be digits or strings,</li> - * <li>strings are checked for well-known qualifiers and the qualifier ordering is used for version ordering. - * Well-known qualifiers (case insensitive) are:<ul> - * <li><code>alpha</code> or <code>a</code></li> - * <li><code>beta</code> or <code>b</code></li> - * <li><code>milestone</code> or <code>m</code></li> - * <li><code>rc</code> or <code>cr</code></li> - * <li><code>snapshot</code></li> - * <li><code>(the empty string)</code> or <code>ga</code> or <code>final</code></li> - * <li><code>sp</code></li> - * </ul> - * Unknown qualifiers are considered after known qualifiers, with lexical order (always case insensitive), - * </li> - * <li>a hyphen usually precedes a qualifier, and is always less important than something preceded with a dot.</li> + * <li> + * string qualifiers are ordered lexically, with the following exceptions: + * <ul> + * <li> 'snapshot' < '' < 'sp' </li> + * </ul> + * and alias => replacement: + * <ul> + * <li> 'a' => 'alpha' </li> + * <li> 'b' => 'beta' </li> + * <li> 'm' => 'milestone' </li> + * <li> 'cr' => 'rc' </li> + * <li> 'final' = 'ga' = 'release' => '' </li> + * </ul> + * </li> + * <li> + * following semver rules is encouraged, and some qualifiers are discouraged: + * <ul> + * <li> the usage of 'CR' qualifier is discouraged, use 'RC' instead. </li> Review Comment: The usage of 'CR' qualifier is discouraged. Use 'RC' instead. and similarly below ########## maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java: ########## @@ -91,7 +112,7 @@ /** * Represents a numeric item in the version item list that can be represented with an int. */ - private static class IntItem + public static class IntItem Review Comment: This should not need to be public ########## maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java: ########## @@ -267,7 +288,7 @@ public String toString() /** * Represents a numeric item in the version item list. */ - private static class BigIntegerItem + public static class BigIntegerItem Review Comment: private ########## maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java: ########## @@ -41,19 +42,34 @@ * <code>1.0alpha1 => [1, 0, alpha, 1]</code></li> * <li>unlimited number of version components,</li> * <li>version components in the text can be digits or strings,</li> - * <li>strings are checked for well-known qualifiers and the qualifier ordering is used for version ordering. - * Well-known qualifiers (case insensitive) are:<ul> - * <li><code>alpha</code> or <code>a</code></li> - * <li><code>beta</code> or <code>b</code></li> - * <li><code>milestone</code> or <code>m</code></li> - * <li><code>rc</code> or <code>cr</code></li> - * <li><code>snapshot</code></li> - * <li><code>(the empty string)</code> or <code>ga</code> or <code>final</code></li> - * <li><code>sp</code></li> - * </ul> - * Unknown qualifiers are considered after known qualifiers, with lexical order (always case insensitive), - * </li> - * <li>a hyphen usually precedes a qualifier, and is always less important than something preceded with a dot.</li> + * <li> + * string qualifiers are ordered lexically, with the following exceptions: Review Comment: String ########## maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java: ########## @@ -41,19 +42,34 @@ * <code>1.0alpha1 => [1, 0, alpha, 1]</code></li> * <li>unlimited number of version components,</li> * <li>version components in the text can be digits or strings,</li> - * <li>strings are checked for well-known qualifiers and the qualifier ordering is used for version ordering. - * Well-known qualifiers (case insensitive) are:<ul> - * <li><code>alpha</code> or <code>a</code></li> - * <li><code>beta</code> or <code>b</code></li> - * <li><code>milestone</code> or <code>m</code></li> - * <li><code>rc</code> or <code>cr</code></li> - * <li><code>snapshot</code></li> - * <li><code>(the empty string)</code> or <code>ga</code> or <code>final</code></li> - * <li><code>sp</code></li> - * </ul> - * Unknown qualifiers are considered after known qualifiers, with lexical order (always case insensitive), - * </li> - * <li>a hyphen usually precedes a qualifier, and is always less important than something preceded with a dot.</li> + * <li> + * string qualifiers are ordered lexically, with the following exceptions: + * <ul> + * <li> 'snapshot' < '' < 'sp' </li> + * </ul> + * and alias => replacement: + * <ul> + * <li> 'a' => 'alpha' </li> + * <li> 'b' => 'beta' </li> + * <li> 'm' => 'milestone' </li> + * <li> 'cr' => 'rc' </li> + * <li> 'final' = 'ga' = 'release' => '' </li> + * </ul> + * </li> + * <li> + * following semver rules is encouraged, and some qualifiers are discouraged: + * <ul> + * <li> the usage of 'CR' qualifier is discouraged, use 'RC' instead. </li> + * <li> the usage of 'final', 'ga', 'release' qualifiers is discouraged, use no qualifier instead. </li> + * <li> the usage of 'SP' qualifier is discouraged, use version increment instead. </li> + * </ul> + * for other qualifiers, natural ordering is used without the need to hard code strings: Review Comment: delete "without the need to hard code strings:" as that refers to implementation details and thus doesn;t need to be in the API doc > ComparableVersion vs versions with custom qualifiers > ---------------------------------------------------- > > Key: MNG-7559 > URL: https://issues.apache.org/jira/browse/MNG-7559 > Project: Maven > Issue Type: Bug > Affects Versions: 3.8.3 > Reporter: Andrzej Jarmoniuk > Priority: Major > Attachments: image-2022-10-22-18-22-11-591.png > > > Since I know that ComparableVersion was brought to Maven from > versions-maven-plugin, it turns out the bug described here: > https://github.com/mojohaus/versions-maven-plugin/issues/744 > also exists in maven, at least in 3.8.3. > According to the maven version spec, versions containing a qualifier should > be treated as less major than the same versions without the qualifier. > Currently it's only the case for a few "standard" qualifiers, e.g. "-rc*", > "-alpha", etc. > However, it looks like "2.3-pfd" is deemed less major than "2.3". > {code:java} > @Test > public void testComparableVersionWithCustomQualifier() > { > assertThat( new ComparableVersion( "2.3" ).compareTo( new > ComparableVersion( "2.3-pfd" ) ), > greaterThan( 0 ) ); > } > {code} -- This message was sent by Atlassian Jira (v8.20.10#820010)