Hiteshsai007 commented on PR #12416:
URL: https://github.com/apache/maven/pull/12416#issuecomment-5092820604
Thanks for the careful re-review @gnodet — all three points are addressed.
Rather than patching on top, I rebuilt the branch from a clean upstream base
and re-applied only the functional change, since two of the three findings were
artifacts of my editor reformatting files it touched.
### 1. Test indentation reverted to 4 spaces
You were right that the whole class had been re-indented to 8 spaces.
`DefaultModelBuilderTest.java` is now byte-identical to upstream except for the
added test method and one new import (`DependencyManagement`).
### 2. Out-of-scope formatting changes removed
The ~47 unrelated hunks in `DefaultModelBuilder.java` (javadoc/comment
rewrapping, record body splitting, `@param` alignment) are gone. That file is
now +42/-11, all of it inside `transformFileToRaw` and the new helper. Total
diff is 174 insertions across 5 files, of which 84 are the three new test POMs.
### 3. `testDuplicateProfileIdsRetainActivations` restored
Dropped because my diff rewrote the entire class body. The rebase brings it
back — it now sits alongside the new test rather than being replaced by it.
### The fix itself (unchanged from the version you approved in acbea96)
`transformFileToRaw` only iterated `model.getDependencies()`, so a BOM
subproject listing sibling reactor modules under `<dependencyManagement>`
without a `<version>` never had those versions inferred, and they were missing
from the raw and consumer POM. The same `inferDependencyVersion` /
`inferDependencyGroupId` logic now runs over
`model.getDependencyManagement().getDependencies()`:
- The shared loop is extracted into a private `inferDependencies(Model,
List<Dependency>, List<Dependency>)` helper returning whether anything changed.
- `depsChanged` and `managedDepsChanged` are tracked separately, so
`builder.dependencies(newDeps)` is only called when the regular dependency list
actually changed — no identical-list copy when only `dependencyManagement` was
modified.
- When nothing is inferred, the original `Model` instance is returned
unchanged, as before.
`testBomDependencyManagementVersionInference` registers a sibling module in
the reactor, runs `transformFileToRaw` on a BOM model whose managed dependency
omits its version, and asserts `assertEquals("1.0-SNAPSHOT",
managedDep.getVersion(), ...)`.
### Note on the rebase base
The branch is rebased onto `af74c4e5` rather than current `master`. It
merges into today's `master` with zero conflicts and I confirmed both test
methods are present in the merge result. Happy to rebase onto the exact tip if
CI needs it.
--
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]
