elharo commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1034705734
##########
maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -40,22 +41,37 @@
* <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>
Review Comment:
Why both = and => ?
Let's be consistent
##########
maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -40,22 +41,37 @@
* <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>
Review Comment:
These are all case sensitive, right?
##########
maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java:
##########
@@ -217,13 +217,13 @@ public void testVersionComparing() {
*/
@Test
public void testMng5568() {
- String a = "6.1.0";
+ String a = "6.1H.5-beta"; // this is the unusual version string, with
'H' in the middle
String b = "6.1.0rc3";
- String c = "6.1H.5-beta"; // this is the unusual version string, with
'H' in the middle
+ String c = "6.1.0";
- checkVersionsOrder(b, a); // classical
- checkVersionsOrder(b, c); // now b < c, but before MNG-5568, we had b
> c
- checkVersionsOrder(a, c);
+ checkVersionsOrder(a, b); // now H < RC as of MNG-7559
Review Comment:
Local variables here obscure the intent. I almost missed what changed here.
Inline them.
##########
maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -40,22 +41,37 @@
* <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', and 'release' qualifiers is
discouraged. Use no qualifier instead. </li>
+ * <li> The usage of 'SP' qualifier is discouraged. Increment the patch
version instead. </li>
Review Comment:
SP or sp?
##########
maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -304,23 +320,24 @@ public String toString() {
* Represents a string in the version item list, usually a qualifier.
*/
private static class StringItem implements Item {
- private static final List<String> QUALIFIERS =
- Arrays.asList("alpha", "beta", "milestone", "rc", "snapshot",
"", "sp");
+ // 'pre-release' < 'snapshot' < 'release'
Review Comment:
I don't see how this comment applies to the field. They have different
contents.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
