Edit version comparison

[elharo]

Mostly just wording, but also remove warning about future enforcement that is unlikely to happen.

[cstamas]

Nope. The refd URL points to maven-artifact spec, that is known to differ from this one (see resolver/maven issues). In Maven 4 the Resolver Version is becoming "the standard" (as new Maven4 API delegates to it), and maven-artifact module is put under compat/, as it is being deprecated with many other things there.

Goal is to stop this current split-brain situation, and duplication of this very important aspect of Maven. Moreover, maven-artifact class Artifact is basically a mixed bag of dependencies (has scope etc) and artifacts. That thing should be just forgotten.

[cstamas]

Or, align the site page with Resolver GenericVersion, then it could be fine to point there.

But the goal is to stop having this duality, duplication and keep trying to align these two classes, aside maven-artifact one is being totally overloaded with other things as well. Just stop pouring energy into it.

[elharo]

Yes, the goal here is precisely to stop the split brain problem where different docs say different things and sometimes contradict each other. The page in question at https://maven.apache.org/pom.html#Version_Order_Specification is the primary Maven documentation from the maven-site repository. It is not tied to maven-artifact, maven-resolver, or any other plugin or particular code. That's important since this is used and depended on by multiple projects. Most importantly this is user documentation for anyone who needs to understand why a dependency tree is resolving the way it is.

It is also important to provide a normative spec for anyone implementing this. For historical reasons, this is a little backwards and some of this is reverse engineered from what the code has been doing for over two decades now rather than planned out up front. Ideally we would have started with a clear and unambiguous spec and built the implementation from that.

Going forward, any additional implementations should start from the spec rather than trying to duplicate code or algorithms. (Yes, other implementations do exist outside the maven project.) I also want to prevent unconsidered changes to the algorithm that sneak in through random PRs. Example: min/max version specifiers that got added 12 years ago, introduced a new supply chain attack, and don't seem to have ever been documented. :-(

[gnodet]

AI Review — PR #1714: Edit version comparison

Thanks for working on improving the version comparison documentation, @elharo!

I spotted a few issues that should be addressed:

🔴 Broken HTML anchor tag (high)

File: maven-resolver-util/src/main/java/org/eclipse/aether/util/version/package-info.java, line 30

The HTML tag <href='...'> is missing the element name — it should be <a href='...'>. As written, the browser will not render a clickable link.

⚠️ Link target may be misleading (medium)

The linked page documents maven-artifact's ComparableVersion behavior, which differs from maven-resolver's GenericVersion. Maintainer @cstamas flagged this concern in the discussion and it appears unresolved. It may be worth linking to the resolver-specific specification instead, or clarifying the relationship in the Javadoc text.

💡 Minor: Space before colon (low)

"These rules are summarized here :" — the space before the colon is unconventional in English.

---

🤖 This review was generated by ForgeBot using a maker/checker pattern (reviewer + independent verifier). All findings were independently confirmed.

[elharo]

Fixed the typos

[gnodet]

Thanks for the quick fix, @elharo! The HTML anchor tag and the spacing are both fixed now. 👍

The remaining question from the original review is the link target — https://maven.apache.org/pom.html#Version_Order_Specification documents maven-artifact's ComparableVersion, while this package implements GenericVersion in maven-resolver. If the two specs are equivalent, a brief note in the Javadoc saying so would help readers; if they diverge, the link may be misleading. I'll leave that call to the maintainers.

---

🤖 This follow-up was generated by ForgeBot. Reviewing new commits on a previously reviewed PR.

[gnodet]

Review Summary

Thanks for the documentation improvements, @elharo. The prose edits throughout are clear improvements to readability and grammar — "serves as a" (was "serves the purpose of"), adding the missing article "is a universal implementation", "in the ROOT locale" (was "in terms of the ROOT locale"), and the various other wording fixes all read better.

I have one substantive concern and one wording suggestion:

Spec Conformance Concern (medium)

The PR positions the inline rules as a summary of the linked Version Order Specification, but that specification documents maven-artifact's ComparableVersion behavior. As @cstamas noted earlier on this PR, that spec "is known to differ" from the resolver's GenericVersion.

Specifically, the inline rules document features absent from the linked spec — for example, the min and max special segments and the ordering rule max > numeric > string > qualifier > min. This makes the framing "These rules are summarized here:" factually inaccurate, since the inline list is a superset of the linked specification, not a summary of it.

A safer framing might be: "The full rules for this implementation are:" — which avoids asserting that the inline list is merely a summary of the linked spec while still referencing it for context.

Positive Notes

• Removal of "None of these are enforced, but in future implementations they probably will be" is a net improvement — that warning was speculative and years old.
• All grammar fixes are correct and improve readability.

---

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet