Skip to content

Fix stale Javadoc and GAV typo in external-profile tests (follow-up to #13141) - #13156

Merged
gnodet merged 2 commits into
apache:masterfrom
gnodet:fix/13100-followup-javadoc-gav
Sep 17, 2026
Merged

gnodet merged 2 commits into
apache:masterfrom
gnodet:fix/13100-followup-javadoc-gav

Conversation

@gnodet

@gnodet gnodet commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #13141, fixing three issues found during review of #13155.

  • DefaultModelBuilderTest: update resolvedProfilesRequest() Javadoc — the sentence "their profiles contribute no repositories" is now false for JDK/OS/activeByDefault-activated profiles; fix active-by-default-profile resolvedSource GAV typo (org.apache.maven.test, matching the fixture and existing test convention)
  • active-by-default-profile.xml: fix groupId plural → singular (org.apache.maven.test) to match the rest of the poms/factory/ fixtures and the test coordinate hint
  • ExternalModelProfileActivationTest (compat): update class-level Javadoc — same stale sentence

- DefaultModelBuilderTest: update resolvedProfilesRequest() Javadoc
  (stale 'contribute no repositories'); fix active-by-default-profile
  resolvedSource GAV (org.apache.maven.test, matching fixture and
  existing test convention)
- active-by-default-profile.xml: fix groupId plural→singular to match
  the rest of the poms/factory fixtures and the test coordinate hint

Found during review of apache#13155.
@gnodet gnodet added the bug Something isn't working label Sep 16, 2026
@gnodet gnodet added this to the 4.1.0 milestone Sep 16, 2026
@gnodet gnodet added documentation Improvements or additions to documentation and removed bug Something isn't working labels Sep 16, 2026

@gnodet-bot gnodet-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Javadoc update to resolvedProfilesRequest() is accurate and well-worded. The POM fixture fix (removing the errant s: org.apache.maven.testsorg.apache.maven.test) is correct.

However the corresponding change to the test's resolvedSource GAV goes in the wrong direction — see inline.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

@gnodet-bot gnodet-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous finding addressed: commit 5764d1b8 correctly reverts the resolvedSource GAV to org.apache.maven.test (no s), consistent with all other call sites in the class.

The two remaining changes are clean:

  • resolvedProfilesRequest() Javadoc accurately reflects the current behavior (repositories from legitimately-active JDK/OS/activeByDefault profiles are honored at CONSUMER_DEPENDENCY level, as implemented in #13100)
  • active-by-default-profile.xml groupId fix (org.apache.maven.testsorg.apache.maven.test) aligns the fixture with the test coordinates

This review was generated by an AI agent, Hermès on behalf of @gnodet.

@gnodet
gnodet merged commit fb1aba8 into apache:master Sep 17, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants