kwin commented on code in PR #260:
URL:
https://github.com/apache/jackrabbit-filevault/pull/260#discussion_r1051910062
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/Version.java:
##########
@@ -135,44 +136,40 @@ public String[] getNormalizedSegments() {
}
/**
- * Compares this version to the given one, segment by segment without any
special
- * "SNAPSHOT" handling.
+ * Compares this version to the given one. The comparison is compatible to
the ordering used by
+ * <a href="https://maven.apache.org/";>Apache Maven</a>. It version
consists normally from 3 numbers -
Review Comment:
```suggestion
* <a href="https://maven.apache.org/";>Apache Maven</a>. A version
usually consists out of three numerical parts separated by dot -
```
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/Version.java:
##########
@@ -135,44 +136,40 @@ public String[] getNormalizedSegments() {
}
/**
- * Compares this version to the given one, segment by segment without any
special
- * "SNAPSHOT" handling.
+ * Compares this version to the given one. The comparison is compatible to
the ordering used by
+ * <a href="https://maven.apache.org/";>Apache Maven</a>. It version
consists normally from 3 numbers -
+ * major version, minor version and patch level, and can be followed by a
dash and a qualifier like SNAPSHOT.
+ * Version numbers can also consist of fewer or more numbers.
+ * If the comparison is not resolved by comparing the numbers, the
algorith resorts to the qualifier - see
Review Comment:
This sentence feels fuzzy, I would just link out to the version order
specification from Maven:
https://maven.apache.org/pom.html#version-order-specification
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/Version.java:
##########
@@ -135,44 +136,40 @@ public String[] getNormalizedSegments() {
}
/**
- * Compares this version to the given one, segment by segment without any
special
- * "SNAPSHOT" handling.
+ * Compares this version to the given one. The comparison is compatible to
the ordering used by
+ * <a href="https://maven.apache.org/";>Apache Maven</a>. It version
consists normally from 3 numbers -
+ * major version, minor version and patch level, and can be followed by a
dash and a qualifier like SNAPSHOT.
+ * Version numbers can also consist of fewer or more numbers.
+ * If the comparison is not resolved by comparing the numbers, the
algorith resorts to the qualifier - see
+ * <a
href="https://cwiki.apache.org/confluence/display/MAVENOLD/Versioning";>"Versioning"
on Maven Wiki</a>
Review Comment:
This resource is really outdated, rather link something more recent like
https://maven.apache.org/pom.html#version-order-specification
##########
vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/VersionRangeTest.java:
##########
@@ -105,9 +105,18 @@ public void testRangeWithEmptyVersionParameter() {
assertFalse("[1.0,2.0] excludes empty version",
vr.isInRange(Version.EMPTY));
}
+ /**
+ * A snapshot precedes the released version, so 1.0-SNAPSHOT not contained
in the range [1.0,2.0)
+ * - but the bound can be a snapshot.
Review Comment:
```suggestion
* - but the boundary can be a snapshot.
```
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/Version.java:
##########
@@ -135,44 +136,40 @@ public String[] getNormalizedSegments() {
}
/**
- * Compares this version to the given one, segment by segment without any
special
- * "SNAPSHOT" handling.
+ * Compares this version to the given one. The comparison is compatible to
the ordering used by
+ * <a href="https://maven.apache.org/";>Apache Maven</a>. It version
consists normally from 3 numbers -
+ * major version, minor version and patch level, and can be followed by a
dash and a qualifier like SNAPSHOT.
+ * Version numbers can also consist of fewer or more numbers.
Review Comment:
```suggestion
* Version numbers can also consist of fewer or more parts (numerical,
string, ....).
```
##########
vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/VersionRangeTest.java:
##########
@@ -105,9 +105,18 @@ public void testRangeWithEmptyVersionParameter() {
assertFalse("[1.0,2.0] excludes empty version",
vr.isInRange(Version.EMPTY));
}
+ /**
+ * A snapshot precedes the released version, so 1.0-SNAPSHOT not contained
in the range [1.0,2.0)
Review Comment:
```suggestion
* A snapshot precedes the released version, so 1.0-SNAPSHOT is not
contained in the range [1.0,2.0)
```
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/Version.java:
##########
@@ -135,44 +136,40 @@ public String[] getNormalizedSegments() {
}
/**
- * Compares this version to the given one, segment by segment without any
special
- * "SNAPSHOT" handling.
+ * Compares this version to the given one. The comparison is compatible to
the ordering used by
+ * <a href="https://maven.apache.org/";>Apache Maven</a>. It version
consists normally from 3 numbers -
+ * major version, minor version and patch level, and can be followed by a
dash and a qualifier like SNAPSHOT.
+ * Version numbers can also consist of fewer or more numbers.
+ * If the comparison is not resolved by comparing the numbers, the
algorith resorts to the qualifier - see
+ * <a
href="https://cwiki.apache.org/confluence/display/MAVENOLD/Versioning";>"Versioning"
on Maven Wiki</a>
+ * for details.
*
* <pre>
- * Examples:
+ * Some examples:
* "1" < "2"
* "1.0" < "2"
* "2.0.1" < "2.1"
* "2.1" < "2.1.1"
* "2.9" < "2.11"
- * "2.1" < "2.1-SNAPSHOT"
- * "2.1" < "2.1-R1234556"
- * "2.1-R12345" < "2.1-SNAPSHOT"
+ * "2.1-SNAPSHOT" < "2.1"
+ * "2.1-RC1" < "2.1"
+ * "2.1-RC1" < "2.1-SNAPSHOT"
* </pre>
*
- * @param o the other version
+ * Please note that this comparison does not use the exact segmentation
presented in {@link #getNormalizedSegments()},
+ * but applies the maven comparison algorithm to the string representation
{@link #toString()}.
+ *
+ * @param o the other version, not null
* @return a negative integer, zero, or a positive integer as this version
* is less than, equal to, or greater than the specified version.
+ *
+ * @see "https://cwiki.apache.org/confluence/display/MAVENOLD/Versioning";
+ * @see "https://semver.org/spec/v1.0.0.html";
Review Comment:
This is an invalid syntax:
https://docs.oracle.com/en/java/javase/17/docs/specs/javadoc/doc-comment-spec.html#see
##########
vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/VersionRangeTest.java:
##########
@@ -105,9 +105,18 @@ public void testRangeWithEmptyVersionParameter() {
assertFalse("[1.0,2.0] excludes empty version",
vr.isInRange(Version.EMPTY));
}
+ /**
+ * A snapshot precedes the released version, so 1.0-SNAPSHOT not contained
in the range [1.0,2.0)
+ * - but the bound can be a snapshot.
+ *
+ * @see
"https://github.com/apache/maven/blob/maven-3.8.6/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/VersionRangeTest.java#L657";
Review Comment:
wrong syntax
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/Version.java:
##########
@@ -135,44 +136,40 @@ public String[] getNormalizedSegments() {
}
/**
- * Compares this version to the given one, segment by segment without any
special
- * "SNAPSHOT" handling.
+ * Compares this version to the given one. The comparison is compatible to
the ordering used by
+ * <a href="https://maven.apache.org/";>Apache Maven</a>. It version
consists normally from 3 numbers -
+ * major version, minor version and patch level, and can be followed by a
dash and a qualifier like SNAPSHOT.
+ * Version numbers can also consist of fewer or more numbers.
+ * If the comparison is not resolved by comparing the numbers, the
algorith resorts to the qualifier - see
+ * <a
href="https://cwiki.apache.org/confluence/display/MAVENOLD/Versioning";>"Versioning"
on Maven Wiki</a>
+ * for details.
*
* <pre>
- * Examples:
+ * Some examples:
* "1" < "2"
* "1.0" < "2"
* "2.0.1" < "2.1"
* "2.1" < "2.1.1"
* "2.9" < "2.11"
- * "2.1" < "2.1-SNAPSHOT"
- * "2.1" < "2.1-R1234556"
- * "2.1-R12345" < "2.1-SNAPSHOT"
+ * "2.1-SNAPSHOT" < "2.1"
+ * "2.1-RC1" < "2.1"
+ * "2.1-RC1" < "2.1-SNAPSHOT"
* </pre>
*
- * @param o the other version
+ * Please note that this comparison does not use the exact segmentation
presented in {@link #getNormalizedSegments()},
+ * but applies the maven comparison algorithm to the string representation
{@link #toString()}.
+ *
+ * @param o the other version, not null
* @return a negative integer, zero, or a positive integer as this version
* is less than, equal to, or greater than the specified version.
+ *
+ * @see "https://cwiki.apache.org/confluence/display/MAVENOLD/Versioning";
Review Comment:
This is an invalid syntax:
https://docs.oracle.com/en/java/javase/17/docs/specs/javadoc/doc-comment-spec.html#see
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
