[GitHub] [maven] elharo commented on a diff in pull request #845: [MNG-7559] Fix versions comparison

[GitBox]

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 =&gt; [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 =&gt; [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 =&gt; [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 =&gt; [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 =&gt; [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



-- 
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: issues-unsubscr...@maven.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org