Tarun4201 commented on PR #13254: URL: https://github.com/apache/maven/pull/13254#issuecomment-5799878600
Thanks for the detailed review! I have addressed all three issues in the latest force-push: ### 1. Rebased onto current `master` The old branch contained commits from the MNG-8056 Cygwin fix (already merged via #13248), which was inflating the diff. I created a clean branch directly from the current upstream `master` so the PR now only shows the MNG-6787 changes — 2 files, 1 commit. ### 2. Added unit tests in `impl/maven-impl` Added `DefaultProfileSelectorTest` directly on `org.apache.maven.impl.model.DefaultProfileSelector` (not the legacy compat one) with three cases: - `externalActiveByDefaultSuppressedWhenProfileExplicitlyActivated` — asserts the `activeByDefault` external profile is **not** active when `-P` is used - `externalActiveByDefaultReturnedWhenNoProfileExplicitlyActivated` — asserts the `activeByDefault` external profile **is** active when no `-P` is given - `pomActiveByDefaultSuppressedWhenOtherPomProfileExplicitlyActivated` — regression guard for the existing POM suppression behaviour ### 3. Documented the semantic asymmetry Added inline comments explaining the intentional difference between the two flags: - `activatedPomProfileNotByDefault` — set by **any** POM profile activation (explicit `-P` *or* condition-based activator such as file/property/OS) - `anyProfileExplicitlyActivated` — set only by strict `-P` / `--activate-profiles` This asymmetry is intentional per the MNG-6787 spec: condition-triggered POM profiles suppress POM `activeByDefault` profiles (pre-existing behaviour), but only an explicit `-P` should suppress external (settings.xml) `activeByDefault` profiles. -- 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]
