Use Maven's standard coordinate order and document compact string syntax#54
Use Maven's standard coordinate order and document compact string syntax#54gnodet wants to merge 2 commits into
Conversation
The README showed the shorthand syntax in examples but never explained the format. Add a dedicated reference section covering GAV strings, dependency strings (GASVTCO), the id field, and direct string values. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe 4- and 5-part dependency coordinate parsing in ChangesCoordinate Parsing Fix and Compact Syntax Documentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The dependency coordinate parser was using g:a:v:type:classifier but Maven's standard format puts version last: g:a[:type[:classifier]]:version. Align the parsing order to match Maven conventions and document the compact string syntax in the README. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
gnodet
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@extension/src/main/java/eu/maveniverse/maven/mason/JsonReaderHelper.java`:
- Around line 132-140: The legacy dependency parsing in JsonReaderHelper is
silently remapping 4/5-part strings into different Dependency fields, which can
corrupt existing YAML after upgrades. Update the parsing logic in
JsonReaderHelper so legacy formats like g:a:v:type and g:a:v:type:classifier are
either interpreted in the same field order as before or explicitly rejected with
a clear exception instead of being reassigned to version/type/classifier. Make
sure the behavior matches the expectations of the Jackson reader mapping in the
generated reader path so existing coordinates do not deserialize with swapped
metadata.
In `@README.md`:
- Around line 208-210: Add explicit language tags to the unlabeled fenced code
blocks in the README so markdownlint MD040 is satisfied. Update the fenced
blocks around the groupId:artifactId examples to use a consistent label such as
text or ebnf, and make the same change for the matching block later in the
document; use the README fenced code examples near the groupId:artifactId
patterns to locate them.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e9d6b971-b852-4516-a2d0-ac08db4fd741
📒 Files selected for processing (4)
README.mdextension/src/main/java/eu/maveniverse/maven/mason/JsonReaderHelper.javaextension/src/test/java/eu/maveniverse/maven/mason/YamlParserTest.javaextension/src/test/resources/yaml/dependency-gav.yaml
| if (parts.length == 3) { | ||
| result[3] = parts[2].isEmpty() ? null : parts[2]; // version | ||
| } else if (parts.length == 4) { | ||
| result[4] = parts[2].isEmpty() ? null : parts[2]; // type | ||
| result[3] = parts[3].isEmpty() ? null : parts[3]; // version | ||
| } else if (parts.length == 5) { | ||
| result[4] = parts[2].isEmpty() ? null : parts[2]; // type | ||
| result[5] = parts[3].isEmpty() ? null : parts[3]; // classifier | ||
| result[3] = parts[4].isEmpty() ? null : parts[4]; // version |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Avoid silently reinterpreting legacy 4/5-part dependency strings.
Lines 134-140 turn previously accepted g:a:v:type / g:a:v:type:classifier inputs into different Dependency objects instead of an upgrade-time error. With the caller in extension/src/main/mdo/jackson-reader.vm:262-306 mapping index 3 to version and 4/5 to type/classifier, existing YAML will now deserialize with swapped metadata and no exception. Please either preserve the legacy order for compatibility or reject it explicitly so upgrades fail fast rather than corrupting dependency coordinates.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@extension/src/main/java/eu/maveniverse/maven/mason/JsonReaderHelper.java`
around lines 132 - 140, The legacy dependency parsing in JsonReaderHelper is
silently remapping 4/5-part strings into different Dependency fields, which can
corrupt existing YAML after upgrades. Update the parsing logic in
JsonReaderHelper so legacy formats like g:a:v:type and g:a:v:type:classifier are
either interpreted in the same field order as before or explicitly rejected with
a clear exception instead of being reassigned to version/type/classifier. Make
sure the behavior matches the expectations of the Jackson reader mapping in the
generated reader path so existing coordinates do not deserialize with swapped
metadata.
| ``` | ||
| groupId:artifactId[:version] | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add language tags to fenced code blocks to satisfy markdownlint (MD040).
Line 208 and Line 223 use unlabeled fenced blocks, which triggers lint warnings. Add an explicit language (for example text or ebnf).
Suggested patch
-```
+```text
groupId:artifactId[:version]- +text
groupId:artifactId[[:type[:classifier]]:version][@scope][?]
Also applies to: 223-225
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 208-208: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 208 - 210, Add explicit language tags to the
unlabeled fenced code blocks in the README so markdownlint MD040 is satisfied.
Update the fenced blocks around the groupId:artifactId examples to use a
consistent label such as text or ebnf, and make the same change for the matching
block later in the document; use the README fenced code examples near the
groupId:artifactId patterns to locate them.
Source: Linters/SAST tools
1 participant
Summary
g:a[:type[:classifier]]:versioninstead of the previousg:a:version:type:classifier. This only affects dependencies that specify type or classifier — the commong:a:vandg:a:v@scopeforms are unchanged.idfield, and direct string valuesTest plan
dependency-gav.yamltest fixtures to use the corrected coordinate orderClaude Code on behalf of Guillaume Nodet
🤖 Generated with Claude Code