ascheman commented on PR #405:
URL: 
https://github.com/apache/maven-help-plugin/pull/405#issuecomment-5249993408

   Agree with @pzygielo's diagnosis — won't re-litigate the 
`String.replace("</version>", …)` issue. Since you're planning to redo this 
with real XML, a few things to make sure the rewrite covers, plus one gotcha:
   
   **Beyond the parent case already shown:**
   - *Multi-module reactors* — at an aggregator, `effective-pom` emits all 
modules under one `<projects>` root, so there are N top-level `<project>` 
elements (each needs its own packaging) and many more `</version>` to 
accidentally hit. The fix has to target each project root, not "the" project.
   - *Schema validity* — `<packaging>` is only valid directly under `<project>` 
(after `<version>`), not inside `<parent>`; the current output is 
schema-invalid there, not just noisy. Selecting the project root element(s) 
fixes both.
   
   **Gotcha:** `model.setPackaging("jar")` before serialization won't help — 
`MavenXpp3Writer` omits packaging when it's `"jar"`, so it'd be dropped again. 
A DOM post-process (for each project root lacking a direct-child `<packaging>`, 
insert after `<version>`) is probably cleanest.
   
   **Test:** the current `verify.groovy` (`find('  
<packaging>jar</packaging>')`) would pass even with the duplication bug. Worth 
an IT matrix: (a) a `<parent>` project asserting no `<packaging>` inside 
`<parent>`; (b) a multi-module reactor asserting one project-level 
`<packaging>` per module and none on deps/plugins; (c) explicit non-jar 
packaging shown once. The count/negative assertions are what catch this.


-- 
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]

Reply via email to