[
https://issues.apache.org/jira/browse/MNG-7559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17633753#comment-17633753
]
ASF GitHub Bot commented on MNG-7559:
-------------------------------------
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.
> ComparableVersion vs versions with custom qualifiers
> ----------------------------------------------------
>
> Key: MNG-7559
> URL: https://issues.apache.org/jira/browse/MNG-7559
> Project: Maven
> Issue Type: Bug
> Affects Versions: 3.8.3
> Reporter: Andrzej Jarmoniuk
> Priority: Major
> Attachments: image-2022-10-22-18-22-11-591.png
>
>
> Since I know that ComparableVersion was brought to Maven from
> versions-maven-plugin, it turns out the bug described here:
> https://github.com/mojohaus/versions-maven-plugin/issues/744
> also exists in maven, at least in 3.8.3.
> According to the maven version spec, versions containing a qualifier should
> be treated as less major than the same versions without the qualifier.
> Currently it's only the case for a few "standard" qualifiers, e.g. "-rc*",
> "-alpha", etc.
> However, it looks like "2.3-pfd" is deemed less major than "2.3".
> {code:java}
> @Test
> public void testComparableVersionWithCustomQualifier()
> {
> assertThat( new ComparableVersion( "2.3" ).compareTo( new
> ComparableVersion( "2.3-pfd" ) ),
> greaterThan( 0 ) );
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
