consistency on Maven 4 vs 3 vs 2 references - #12642
Conversation
gnodet
left a comment
There was a problem hiding this comment.
The POM naming changes are correct and consistent with the existing convention (e.g., "Maven 4 API :: ...", "Maven 4 ..."). However, renaming the CLUSTER_PATTERNS keys in ReactorGraph.java breaks two downstream code paths that derive identifiers by stripping spaces from those keys.
Issue 1: Switch statement hyperlinks (line 189-194)
The switch at line 189 matches on clusterName, which is the CLUSTER_PATTERNS key with spaces stripped (line 184: key.replaceAll("\\s+", "")). After the rename:
| Key | Stripped (new) | Switch case (old) | Match? |
|---|---|---|---|
Maven 4 API |
Maven4API |
MavenAPI |
❌ |
Maven Resolver 2 |
MavenResolver2 |
MavenResolver |
❌ |
Maven 4 Implementation |
Maven4Implementation |
MavenImplementation |
❌ |
Maven 3 Compatibility |
Maven3Compatibility |
MavenCompatibility |
❌ |
All four cases silently miss → default: prefix = null → the high-level graph loses all hyperlinks.
Issue 2: Rank constraint orphan nodes (lines 255-257)
The rank constraint still references old names (MavenResolver, MavenAPI, MavenImplementation), but the actual graph nodes are now MavenResolver2, Maven4API, Maven4Implementation. This creates orphan nodes, breaking the graph layout.
Fix
Update both the switch cases and the rank constraint mutNode() calls to match the new space-stripped names.
Minor
compat/maven-compat/pom.xml — the <description> still says "Maven2" (no space) while <name> now says "Maven 2" (with space). In a naming-consistency PR, this could also be updated.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of gnodet
| <name>Maven Compat (deprecated)</name> | ||
| <description>Deprecated Maven2 classes maintained as compatibility layer.</description> | ||
| <name>Maven 2 Compat (deprecated)</name> | ||
| <description>Deprecated Maven2 classes, maintained as compatibility layer.</description> |
There was a problem hiding this comment.
we're inconsistent about this but I really prefer "Maven 2" over "Maven2"
There was a problem hiding this comment.
yes, while at it, let's update the description
consistency on Maven 4 vs 3 vs 2 references
https://maven.apache.org/ref/4.0.0-rc-6/
Maven 4 API and Impl
Maven 3 compatibility modules
maven-compat is Maven 2 compat