Hiteshsai007 commented on PR #12744: URL: https://github.com/apache/maven/pull/12744#issuecomment-5312021337
Thanks for the detailed review and approval, @gnodet! I've addressed the three non-blocking observations in the latest commit: ### 1. Duplicate dependency guard ✅ Replaced the naive `List.addAll()` merging with **key-based deduplication** using `LinkedHashMap` for all three merge paths in `inlinePackagingActivatedProfiles`: - **Dependencies**: Deduplicated by `groupId:artifactId:type:classifier` — existing model dependencies take precedence over profile duplicates via `putIfAbsent` - **Managed dependencies**: Same key-based deduplication - **Repositories**: Deduplicated by repository `id` ### 2. Test coverage for DM/repos paths ✅ Added three new unit tests to exercise the previously untested merge paths: - `testInlinePackagingActivatedProfilesDependencyManagement` — verifies dependency management entries are correctly inlined from packaging-only profiles - `testInlinePackagingActivatedProfilesRepositories` — verifies repositories are correctly inlined - `testInlinePackagingActivatedProfilesDeduplication` — verifies the duplicate guard (model dependency at version 1.0 takes precedence over profile duplicate at version 2.0) ### 3. Javadoc completeness ✅ This was already documented in the existing Javadoc — the method-level doc at lines 475–477 explicitly states: *"Profiles with a non-matching packaging activation are dropped entirely, since they can never activate for this artifact's fixed packaging and their presence would block model version downgrade to 4.0.0."* Additionally, I widened `transformBom` visibility from `private static` to `static` (package-private) to match `transformNonPom` and `transformPom`, fixing a pre-existing compilation error in `testBomPackagingActivatedProfilesArePreserved`. All 14 tests pass. 🟢 -- 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]
