Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MNG-8588] Add activated profiles to Project interface #2132

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

gnodet
Copy link
Contributor

@gnodet gnodet commented Feb 26, 2025

Based on #2119

  • [MNG-8588] Add activated profiles to Project interface
  • [MNG-8588] Add tests for getInjectedProfileIds()
  • Define getActiveProfiles() instead
  • Add getProfiles methods and fix location tracking on settings

@gnodet
Copy link
Contributor Author

gnodet commented Feb 26, 2025

@Giovds I went ahead and added the get[All][Active]Profiles() methods on Project.
I've locally updated m-help-p to use those and had to fix location tracking in settings for the real source to be displayed (instead of just pom, external or settings.xml).
Next step would be to actually remove the source field from Profile...

Comment on lines +242 to +267
/**
* Returns all profiles defined in this project.
* <p>
* This method returns only the profiles defined directly in the current project's POM
* and does not include profiles from parent projects.
*
* @return a non-null, possibly empty list of profiles defined in this project
* @see Profile
* @see #getAllProfiles()
*/
@Nonnull
List<Profile> getProfiles();

/**
* Returns all profiles defined in this project and all of its parent projects.
* <p>
* This method traverses the parent hierarchy and includes profiles defined in parent POMs.
* The returned list contains profiles from the current project and all of its ancestors in
* the project inheritance chain.
*
* @return a non-null, possibly empty list of all profiles from this project and its parents
* @see Profile
* @see #getProfiles()
*/
@Nonnull
List<Profile> getAllProfiles();
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm trying to think of a different name for the getAll(Active)Profiles() methods, as it might be interpreted the same as getProfiles(). I was thinking something alone the lines of getAll(Active)ProfilesIncludingInherited(), or rename get(Active)Profiles() to something like getCurrent(Active)Profiles()

Copy link
Contributor Author

@gnodet gnodet Feb 26, 2025

Choose a reason for hiding this comment

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

What about:

  • getDeclaredProfiles() / getEffectiveProfiles()
  • getDeclaredActiveProfiles() / getEffectiveActiveProfiles()

The declared would make reference to only profiles on the current project, while effective would make reference to profiles from the project and it's hierarchy, similar to the effective model which is computed with the parents merged into it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thats a good one. As it also follows the terminology we use within other parts of the code base and documentation. It does require some knowledge of our domain as consumer of the API but the docs clearly describe the intent

Comment on lines +199 to +208
public List<Profile> getAllActiveProfiles() {
List<org.apache.maven.model.Profile> activeProfiles = new ArrayList<>();
for (MavenProject project = this.project; project != null; project = project.getParent()) {
activeProfiles.addAll(project.getActiveProfiles());
}
return activeProfiles.stream()
.map(org.apache.maven.model.Profile::getDelegate)
.toList();
}

Copy link
Contributor

Choose a reason for hiding this comment

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

You could make the methods a one liner, if you'd like with something like this I think:

    return Stream.iterate(this.project, Objects::nonNull, MavenProject::getParent)
                 .flatMap(project -> project.getActiveProfiles().stream())
                 .map(org.apache.maven.model.Profile::getDelegate)
                 .toList();

…efaultProject.java

Co-authored-by: Giovanni van der Schelde <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants