You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/10/22 15:37:06 UTC
[GitHub] [maven] sultan opened a new pull request, #845: Fix versions comparison
sultan opened a new pull request, #845:
URL: https://github.com/apache/maven/pull/845
Fix versions comparison
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on PR #845:
URL: https://github.com/apache/maven/pull/845#issuecomment-1312576539
* [x] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1020922989
##########
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:
+ * <ul>
+ * <li>alpha = a < beta = b < ea < milestone = m < preview < rc = cr < snapshot < '' = final = ga = release < sp</li>
Review Comment:
removed
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on PR #845:
URL: https://github.com/apache/maven/pull/845#issuecomment-1304817147
added regression test cases
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] elharo merged pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
elharo merged PR #845:
URL: https://github.com/apache/maven/pull/845
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1002523615
##########
maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -354,15 +354,15 @@ public String toString()
implements Item
{
private static final List<String> QUALIFIERS =
- Arrays.asList( "alpha", "beta", "milestone", "rc", "snapshot", "", "sp" );
+ Arrays.asList( "alpha", "beta", "milestone", "preview", "rc", "snapshot", "", "sp" );
private static final Properties ALIASES = new Properties();
static
{
- ALIASES.put( "ga", "" );
+ ALIASES.put( "cr", "rc" );
ALIASES.put( "final", "" );
+ ALIASES.put( "ga", "" );
ALIASES.put( "release", "" );
- ALIASES.put( "cr", "rc" );
}
Review Comment:
not functional but makes the tag ordering intention clearer
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #845:
URL: https://github.com/apache/maven/pull/845#issuecomment-1358433880
This merge has been reverted on master.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on PR #845:
URL: https://github.com/apache/maven/pull/845#issuecomment-1312758877
reverted public visibilities back to the previous private when they were
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on PR #845:
URL: https://github.com/apache/maven/pull/845#issuecomment-1330695709
@elharo, fixed javadoc build errors by replacing > with > and < with <
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1002523615
##########
maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -354,15 +354,15 @@ public String toString()
implements Item
{
private static final List<String> QUALIFIERS =
- Arrays.asList( "alpha", "beta", "milestone", "rc", "snapshot", "", "sp" );
+ Arrays.asList( "alpha", "beta", "milestone", "preview", "rc", "snapshot", "", "sp" );
private static final Properties ALIASES = new Properties();
static
{
- ALIASES.put( "ga", "" );
+ ALIASES.put( "cr", "rc" );
ALIASES.put( "final", "" );
+ ALIASES.put( "ga", "" );
ALIASES.put( "release", "" );
- ALIASES.put( "cr", "rc" );
}
Review Comment:
not functional but makes the tag ordering intention clearer: RC < final, ga, release
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1002518719
##########
maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -354,15 +354,15 @@ public String toString()
implements Item
{
private static final List<String> QUALIFIERS =
- Arrays.asList( "alpha", "beta", "milestone", "rc", "snapshot", "", "sp" );
+ Arrays.asList( "alpha", "beta", "milestone", "preview", "rc", "snapshot", "", "sp" );
Review Comment:
preview is used like milestone, so previous to RC
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on PR #845:
URL: https://github.com/apache/maven/pull/845#issuecomment-1304819529
ready for review @michael-o
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on a diff in pull request #845: Fix versions comparison
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1002518336
##########
maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -354,15 +354,15 @@ public String toString()
implements Item
{
private static final List<String> QUALIFIERS =
- Arrays.asList( "alpha", "beta", "milestone", "rc", "snapshot", "", "sp" );
+ Arrays.asList( "alpha", "beta", "milestone", "preview", "rc", "snapshot", "", "sp" );
private static final Properties ALIASES = new Properties();
static
{
- ALIASES.put( "ga", "" );
+ ALIASES.put( "cr", "rc" );
ALIASES.put( "final", "" );
+ ALIASES.put( "ga", "" );
ALIASES.put( "release", "" );
- ALIASES.put( "cr", "rc" );
}
Review Comment:
What is the benefit of this reorder? None, no?
##########
maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -354,15 +354,15 @@ public String toString()
implements Item
{
private static final List<String> QUALIFIERS =
- Arrays.asList( "alpha", "beta", "milestone", "rc", "snapshot", "", "sp" );
+ Arrays.asList( "alpha", "beta", "milestone", "preview", "rc", "snapshot", "", "sp" );
Review Comment:
What is the difference between preview and release candidate?
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on PR #845:
URL: https://github.com/apache/maven/pull/845#issuecomment-1312749220
> I'm still digging into the other PRs and the existing spec and code. Pleas bear with me. It's been a while since I looked at this, and I need to make sure all the fiddly little bits are straight in my head.
no emergency, still available to clarify anything.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on PR #845:
URL: https://github.com/apache/maven/pull/845#issuecomment-1355512813
@elharo is it wished to port this to 3.9.x branch? if so a PR is available here:
* PR #887
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1034713665
##########
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:
yes, i'll put a comment that the case does not matter
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1034724519
##########
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:
done
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] elharo commented on pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
elharo commented on PR #845:
URL: https://github.com/apache/maven/pull/845#issuecomment-1336213615
All looks good. I need to copy the PR into my account to run it through Jenkins
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1020919045
##########
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:
+ * <ul>
+ * <li>alpha = a < beta = b < ea < milestone = m < preview < rc = cr < snapshot < '' = final = ga = release < sp</li>
Review Comment:
this is just a mere example of how the alphabetical ordering works. since this seems to be a problem, i will remove them from the 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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1020913707
##########
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:
done
##########
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:
done
##########
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:
done
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1020914924
##########
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:
Maven Versions Plugin has the same ordering issues. They rely on a "copy" of that very same class with public inner classes for their code/fixes to work. making them public here could prevent other projects having an out of date copy of their own.
##########
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:
same
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] elharo commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1034713031
##########
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:
both
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1034726107
##########
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:
done
##########
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:
done
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1034719316
##########
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:
fair, i'll use =>
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] elharo commented on pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
elharo commented on PR #845:
URL: https://github.com/apache/maven/pull/845#issuecomment-1355537220
Check on the dev mailing list and see of anyone is planning another 3.9 release.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1034721686
##########
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:
fair i'll remove the comment
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on PR #845:
URL: https://github.com/apache/maven/pull/845#issuecomment-1330532766
@elharo rebased and applied code styles with spotless plugin
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1002523419
##########
maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -354,15 +354,15 @@ public String toString()
implements Item
{
private static final List<String> QUALIFIERS =
- Arrays.asList( "alpha", "beta", "milestone", "rc", "snapshot", "", "sp" );
+ Arrays.asList( "alpha", "beta", "milestone", "preview", "rc", "snapshot", "", "sp" );
Review Comment:
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on PR #845:
URL: https://github.com/apache/maven/pull/845#issuecomment-1312748739
thanks for the feedback
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on PR #845:
URL: https://github.com/apache/maven/pull/845#issuecomment-1312761182
> My major concern here is that ea and preview are new. Is it simply the case that their natural order already gives the correct results, both before and after this PR? If so, this is fine.
ea and preview removed and let treated with natural ordering
> I also do not want to expand the public API in this PR. Looking at this code now I see at least one potential refactoring that would change that newly public API. In any case, new public API needs additional thought and discussion. It's a separate issue from fixing this bug.
removed the public visibilities
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1020915067
##########
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:
ok, do i need to make this replacement in both code and website doc ?
##########
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:
ok, do i need to make a replacement in both code and website 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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1020917747
##########
maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -350,26 +371,27 @@ public String toString()
/**
* Represents a string in the version item list, usually a qualifier.
*/
- private static class StringItem
+ public static class StringItem
implements Item
{
- private static final List<String> QUALIFIERS =
- Arrays.asList( "alpha", "beta", "milestone", "rc", "snapshot", "", "sp" );
+ // 'pre-release' < 'snapshot' < 'release'
+ private static final List<String> QUALIFIERS = Arrays.asList( "snapshot", "", "sp" );
Review Comment:
the heart of the fix relies here, so better have this done right!
##########
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:
will be treated in another PR
##########
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>
Review Comment:
done on both sides (code/doc)
##########
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:
+ * <ul>
+ * <li>alpha = a < beta = b < ea < milestone = m < preview < rc = cr < snapshot < '' = final = ga = release < sp</li>
+ * </ul>
+ * </li>
+ * <li>a hyphen usually precedes a qualifier, and is always less important than digits/number, for example
+ * 1.0.RC2 < 1.0-RC3 < 1.0.1 ; but prefer '1.0.0-RC1' over '1.0.0.RC1' </li>
* </ul>
*
* @see <a href="https://cwiki.apache.org/confluence/display/MAVENOLD/Versioning">"Versioning" on Maven Wiki</a>
Review Comment:
changed to refer to the website doc
##########
maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -73,7 +89,12 @@
private ListItem items;
- private interface Item
+ public ListItem getItems()
Review Comment:
will be treated in another PR
##########
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:
+ * <ul>
+ * <li>alpha = a < beta = b < ea < milestone = m < preview < rc = cr < snapshot < '' = final = ga = release < sp</li>
Review Comment:
this is just a mere example of how the alphabetical ordering works. since this seems to be a problem, i will remove them from the doc
##########
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:
i will make a separate PR for this particular problem then. but the problem still needs to be addressed ...
##########
maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -404,27 +426,31 @@ public int getType()
@Override
public boolean isNull()
{
- return ( comparableQualifier( value ).compareTo( RELEASE_VERSION_INDEX ) == 0 );
+ return QUALIFIERS.indexOf( value ) == RELEASE_VERSION_INDEX;
}
/**
- * Returns a comparable value for a qualifier.
- *
- * This method takes into account the ordering of known qualifiers then unknown qualifiers with lexical
- * ordering.
+ * Compare the qualifiers of two artifact versions.
*
- * just returning an Integer with the index here is faster, but requires a lot of if/then/else to check for -1
- * or QUALIFIERS.size and then resort to lexical ordering. Most comparisons are decided by the first character,
- * so this is still fast. If more characters are needed then it requires a lexical sort anyway.
- *
- * @param qualifier
- * @return an equivalent value that can be used with lexical comparison
+ * @param qualifier1 qualifier of first artifact
+ * @param qualifier2 qualifier of second artifact
+ * @return a negative integer, zero, or a positive integer as the first argument is less than, equal to, or
+ * greater than the second.
*/
- public static String comparableQualifier( String qualifier )
+ public static int compareQualifiers( String qualifier1, String qualifier2 )
{
- int i = QUALIFIERS.indexOf( qualifier );
+ int i1 = QUALIFIERS.indexOf( qualifier1 );
+ int i2 = QUALIFIERS.indexOf( qualifier2 );
+
+ // if both pre-release, then use natural lexical ordering
+ if ( i1 == -1 && i2 == -1 )
+ {
+ // alpha < beta < ea < milestone < preview < rc
+ return qualifier1.compareTo( qualifier2 );
+ }
- return i == -1 ? ( QUALIFIERS.size() + "-" + qualifier ) : String.valueOf( i );
+ // 'other qualifier' < 'snapshot' < '' < 'sp'
+ return Integer.compare( i1, i2 );
}
Review Comment:
the heart of the fix relies here, so better have this done right!
##########
maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -404,27 +426,31 @@ public int getType()
@Override
public boolean isNull()
{
- return ( comparableQualifier( value ).compareTo( RELEASE_VERSION_INDEX ) == 0 );
+ return QUALIFIERS.indexOf( value ) == RELEASE_VERSION_INDEX;
}
/**
- * Returns a comparable value for a qualifier.
- *
- * This method takes into account the ordering of known qualifiers then unknown qualifiers with lexical
- * ordering.
+ * Compare the qualifiers of two artifact versions.
*
- * just returning an Integer with the index here is faster, but requires a lot of if/then/else to check for -1
- * or QUALIFIERS.size and then resort to lexical ordering. Most comparisons are decided by the first character,
- * so this is still fast. If more characters are needed then it requires a lexical sort anyway.
- *
- * @param qualifier
- * @return an equivalent value that can be used with lexical comparison
+ * @param qualifier1 qualifier of first artifact
+ * @param qualifier2 qualifier of second artifact
+ * @return a negative integer, zero, or a positive integer as the first argument is less than, equal to, or
+ * greater than the second.
*/
- public static String comparableQualifier( String qualifier )
+ public static int compareQualifiers( String qualifier1, String qualifier2 )
{
- int i = QUALIFIERS.indexOf( qualifier );
+ int i1 = QUALIFIERS.indexOf( qualifier1 );
+ int i2 = QUALIFIERS.indexOf( qualifier2 );
+
+ // if both pre-release, then use natural lexical ordering
+ if ( i1 == -1 && i2 == -1 )
+ {
+ // alpha < beta < ea < milestone < preview < rc
+ return qualifier1.compareTo( qualifier2 );
+ }
- return i == -1 ? ( QUALIFIERS.size() + "-" + qualifier ) : String.valueOf( i );
+ // 'other qualifier' < 'snapshot' < '' < 'sp'
+ return Integer.compare( i1, i2 );
}
Review Comment:
any qualifier other than 'SP' are less than 'release' or ''
##########
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:
will be treated in another PR
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1020915067
##########
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:
ok, do i need to make a replacement? in both code and website 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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1020913627
##########
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:
done.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1020914945
##########
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:
same story, i suppose
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1021508147
##########
maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -404,27 +421,31 @@ public int getType()
@Override
public boolean isNull()
{
- return ( comparableQualifier( value ).compareTo( RELEASE_VERSION_INDEX ) == 0 );
+ return QUALIFIERS.indexOf( value ) == RELEASE_VERSION_INDEX;
}
/**
- * Returns a comparable value for a qualifier.
- *
- * This method takes into account the ordering of known qualifiers then unknown qualifiers with lexical
- * ordering.
+ * Compare the qualifiers of two artifact versions.
*
- * just returning an Integer with the index here is faster, but requires a lot of if/then/else to check for -1
- * or QUALIFIERS.size and then resort to lexical ordering. Most comparisons are decided by the first character,
- * so this is still fast. If more characters are needed then it requires a lexical sort anyway.
- *
- * @param qualifier
- * @return an equivalent value that can be used with lexical comparison
+ * @param qualifier1 qualifier of first artifact
+ * @param qualifier2 qualifier of second artifact
+ * @return a negative integer, zero, or a positive integer as the first argument is less than, equal to, or
+ * greater than the second.
Review Comment:
ok, removed the period
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] elharo commented on pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
elharo commented on PR #845:
URL: https://github.com/apache/maven/pull/845#issuecomment-1336244089
Jenkins passed my fork of this so I'm going to merge.
https://ci-maven.apache.org/job/Maven/job/maven-box/job/maven/job/MNG-7559/
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1034713665
##########
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:
yes
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on PR #845:
URL: https://github.com/apache/maven/pull/845#issuecomment-1330623513
made the suggested change, thanks @elharo
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on PR #845:
URL: https://github.com/apache/maven/pull/845#issuecomment-1312572266
code:
* PR https://github.com/apache/maven/pull/845
documentation:
* PR https://github.com/apache/maven-site/pull/331
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] elharo commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
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
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on PR #845:
URL: https://github.com/apache/maven/pull/845#issuecomment-1312736928
code and documentation on par. waiting for final reviews.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1020914924
##########
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:
Maven Versions Plugin has the same ordering issues. They rely on a "copy" of that very same file with public inner classes for their code/fixes to work. making them public here could prevent other projects having an out of date copy of their own.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on PR #845:
URL: https://github.com/apache/maven/pull/845#issuecomment-1359307317
> This merge has been reverted on master.
A new clean PR was created to reintroduce the fix
* PR #929
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on PR #845:
URL: https://github.com/apache/maven/pull/845#issuecomment-1312730805
added another test case to prove website documentation right
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] elharo commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
elharo commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1020916558
##########
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:
+ * <ul>
+ * <li>alpha = a < beta = b < ea < milestone = m < preview < rc = cr < snapshot < '' = final = ga = release < sp</li>
Review Comment:
ea and preview are new
##########
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>
Review Comment:
Increment the patch version instead.
##########
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:
YAGNI
##########
maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -73,7 +89,12 @@
private ListItem items;
- private interface Item
+ public ListItem getItems()
Review Comment:
why public?
##########
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:
+ * <ul>
+ * <li>alpha = a < beta = b < ea < milestone = m < preview < rc = cr < snapshot < '' = final = ga = release < sp</li>
+ * </ul>
+ * </li>
+ * <li>a hyphen usually precedes a qualifier, and is always less important than digits/number, for example
+ * 1.0.RC2 < 1.0-RC3 < 1.0.1 ; but prefer '1.0.0-RC1' over '1.0.0.RC1' </li>
* </ul>
*
* @see <a href="https://cwiki.apache.org/confluence/display/MAVENOLD/Versioning">"Versioning" on Maven Wiki</a>
Review Comment:
This is out of date and could be removed or pointed at the official docs instea dof the wiki
##########
maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -350,26 +371,27 @@ public String toString()
/**
* Represents a string in the version item list, usually a qualifier.
*/
- private static class StringItem
+ public static class StringItem
Review Comment:
private
##########
maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -73,7 +89,12 @@
private ListItem items;
- private interface Item
+ public ListItem getItems()
+ {
+ return items;
+ }
+
+ public interface Item
Review Comment:
private
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1020914945
##########
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:
same story, i believe
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] elharo commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
elharo commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1021450876
##########
maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -404,27 +421,31 @@ public int getType()
@Override
public boolean isNull()
{
- return ( comparableQualifier( value ).compareTo( RELEASE_VERSION_INDEX ) == 0 );
+ return QUALIFIERS.indexOf( value ) == RELEASE_VERSION_INDEX;
}
/**
- * Returns a comparable value for a qualifier.
- *
- * This method takes into account the ordering of known qualifiers then unknown qualifiers with lexical
- * ordering.
+ * Compare the qualifiers of two artifact versions.
*
- * just returning an Integer with the index here is faster, but requires a lot of if/then/else to check for -1
- * or QUALIFIERS.size and then resort to lexical ordering. Most comparisons are decided by the first character,
- * so this is still fast. If more characters are needed then it requires a lexical sort anyway.
- *
- * @param qualifier
- * @return an equivalent value that can be used with lexical comparison
+ * @param qualifier1 qualifier of first artifact
+ * @param qualifier2 qualifier of second artifact
+ * @return a negative integer, zero, or a positive integer as the first argument is less than, equal to, or
+ * greater than the second.
Review Comment:
nit no period
##########
maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -404,27 +421,31 @@ public int getType()
@Override
public boolean isNull()
{
- return ( comparableQualifier( value ).compareTo( RELEASE_VERSION_INDEX ) == 0 );
+ return QUALIFIERS.indexOf( value ) == RELEASE_VERSION_INDEX;
}
/**
- * Returns a comparable value for a qualifier.
- *
- * This method takes into account the ordering of known qualifiers then unknown qualifiers with lexical
- * ordering.
+ * Compare the qualifiers of two artifact versions.
*
- * just returning an Integer with the index here is faster, but requires a lot of if/then/else to check for -1
- * or QUALIFIERS.size and then resort to lexical ordering. Most comparisons are decided by the first character,
- * so this is still fast. If more characters are needed then it requires a lexical sort anyway.
- *
- * @param qualifier
- * @return an equivalent value that can be used with lexical comparison
+ * @param qualifier1 qualifier of first artifact
+ * @param qualifier2 qualifier of second artifact
+ * @return a negative integer, zero, or a positive integer as the first argument is less than, equal to, or
+ * greater than the second.
*/
- public static String comparableQualifier( String qualifier )
+ public static int compareQualifiers( String qualifier1, String qualifier2 )
Review Comment:
This is a public API breaking change, and in this case I do expect there's code outside this project depending on this method. In fact, I've probably written code like that myself. I think we need to retain the existing method with the existing behavior in addition to adding this one.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1021509233
##########
maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -404,27 +421,31 @@ public int getType()
@Override
public boolean isNull()
{
- return ( comparableQualifier( value ).compareTo( RELEASE_VERSION_INDEX ) == 0 );
+ return QUALIFIERS.indexOf( value ) == RELEASE_VERSION_INDEX;
}
/**
- * Returns a comparable value for a qualifier.
- *
- * This method takes into account the ordering of known qualifiers then unknown qualifiers with lexical
- * ordering.
+ * Compare the qualifiers of two artifact versions.
*
- * just returning an Integer with the index here is faster, but requires a lot of if/then/else to check for -1
- * or QUALIFIERS.size and then resort to lexical ordering. Most comparisons are decided by the first character,
- * so this is still fast. If more characters are needed then it requires a lexical sort anyway.
- *
- * @param qualifier
- * @return an equivalent value that can be used with lexical comparison
+ * @param qualifier1 qualifier of first artifact
+ * @param qualifier2 qualifier of second artifact
+ * @return a negative integer, zero, or a positive integer as the first argument is less than, equal to, or
+ * greater than the second.
*/
- public static String comparableQualifier( String qualifier )
+ public static int compareQualifiers( String qualifier1, String qualifier2 )
Review Comment:
made a non breakage by re-adding a method with same signature and feature compatible with new method
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on PR #845:
URL: https://github.com/apache/maven/pull/845#issuecomment-1313673933
made 3 changes.
* javadoc without the last dot
* put back a method that returns a string
* fixed test, hopefully
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on PR #845:
URL: https://github.com/apache/maven/pull/845#issuecomment-1304789299
made the items attribute private with a public getter
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1034725131
##########
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:
done
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1034726401
##########
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:
done
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1034713031
##########
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:
both, i'll put a comment that the case does not matter
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] sultan commented on a diff in pull request #845: [MNG-7559] Fix versions comparison
Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #845:
URL: https://github.com/apache/maven/pull/845#discussion_r1034722190
##########
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:
why not, inlining the vars
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org