CAMEL-23973: camel export - Add --parent-pom option#24906
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Review: CAMEL-23973 — camel export --parent-pom option
Overall: Well-implemented feature that addresses a real organizational need for custom parent POMs in exported Camel projects.
What works well
-
Thoughtful Spring Boot BOM handling — When
--parent-pomreplaces the defaultspring-boot-starter-parent, thespring-boot-dependenciesBOM remains in<dependencyManagement>so version management continues to work. The FreeMarker templates use[#if ParentGroupId??]conditionals cleanly. -
Clean layering with
enrichParentPom()— Centralizing the model population inExportBaseCommandand calling it uniformly from all three runtime exporters avoids duplication. Each exporter adds one line (enrichParentPom(model)) and the templates handle the rest. -
Comprehensive coverage — All six POM templates updated, tests for both with/without parent POM across all runtimes, documentation in the export guide with examples, and
application.propertiessupport viacamel.jbang.parentPom. -
<relativePath/>in generated parent block — Prevents Maven from searching parent directories, which is the right default for a remote parent.
Non-blocking observation
Input validation for malformed --parent-pom — The enrichParentPom() method splits on : and silently ignores input with != 3 parts. If a user specifies --parent-pom=com.ourorg:camel-parent (forgetting the version), the export succeeds with exit 0 but the parent POM is never applied. For Spring Boot, this silently falls back to the default spring-boot-starter-parent.
The existing --gav option validates and fails fast:
if (ids.length != 3) {
printer().printErr("--gav must be in syntax: groupId:artifactId:version");
return 1;
}Consider adding similar validation for --parent-pom for consistency:
protected void enrichParentPom(Map<String, Object> model) {
if (parentPom != null) {
String[] parts = parentPom.split(":");
if (parts.length != 3) {
throw new IllegalArgumentException(
"--parent-pom must be in syntax: groupId:artifactId:version, got: " + parentPom);
}
model.put("ParentGroupId", parts[0]);
model.put("ParentArtifactId", parts[1]);
model.put("ParentVersion", parts[2]);
}
}This can be addressed as a follow-up — the feature works correctly for valid input.
Claude Code on behalf of gnodet — AI-generated review
…parseStrictGav Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
|
Thanks for the review @gnodet! Good catch on the silent ignore for malformed Addressed by adding
Also regenerated Claude Code on behalf of davsclaus |
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Re-review after 2nd commit (36f357c): Strict GAV validation via MavenGav.parseStrictGav() — nice 👍
The new commit addresses the observation from my earlier review about silently ignoring malformed --parent-pom values. Changes:
- New
MavenGav.parseStrictGav()— validates exactly 3 colon-separated parts, throwsIllegalArgumentExceptionwith a descriptive message otherwise. Good Javadoc. enrichParentPom()— now usesparseStrictGav()instead of manualsplit(":"), so malformed input throws rather than being silently ignored.- Bonus: unified
--gavvalidation —ExportCamelMain,ExportQuarkus, andExportSpringBootall upgraded to useparseStrictGav()for--gavas well, replacing the manualids.length != 3checks. Consistent.
Minor note: enrichParentPom() lets the IllegalArgumentException propagate uncaught, whereas the --gav validation wraps it in a try/catch with a clean error message. If a user passes --parent-pom foo:bar they'll get a stack trace rather than a friendly message. Not blocking — it's strictly better than silently ignoring, and easily refined later if needed.
My earlier approval stands.
Claude Code on behalf of gnodet — AI-generated review
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 12 tested, 24 compile-only — current: 9 all testedMaveniverse Scalpel detected 36 affected modules (current approach: 9).
|
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Re-review (3rd commit 816a4a11): Early --parent-pom validation with friendly error message.
Good addition — validates --parent-pom early (right after --gav validation) using parseStrictGav() with a user-friendly error message, consistent across all three export commands (CamelMain, Quarkus, SpringBoot). This catches malformed parent POM coordinates before any export work begins, matching the existing --gav validation pattern directly above.
Prior approval stands. ✅
Claude Code on behalf of gnodet — AI-generated review
2 participants
Summary
--parent-pom=groupId:artifactId:versionoption tocamel exportfor setting a custom Maven parent POM in the exported projectspring-boot-starter-parent; thespring-boot-dependenciesBOM in<dependencyManagement>keeps version management intactcamel.jbang.parentPominapplication.propertiesMavenGav.parseStrictGav()for shared strict GAV validation — used by both--gavand--parent-pomvalidation across all export commands--parent-pomearly inexport()with a friendly error message (consistent with--gavvalidation)Test plan
shouldExportWithParentPom— verifies custom parent is set for all runtimes, and Spring Boot BOM is preservedshouldExportWithoutParentPom— verifies default behavior is unchangedClaude Code on behalf of davsclaus
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com