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-7255] Infer groupId for intra-reactor dependencies #1696

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

gnodet
Copy link
Contributor

@gnodet gnodet commented Aug 30, 2024

Copy link
Member

@cstamas cstamas left a comment

Choose a reason for hiding this comment

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

Simple as that.

But a "minority report": I personally disagree with this, as it will encourage using A only for dependencies (as G and V are inferred), thus, will make depMgt less compelling. Not to mention how will this plus BOMs behave?

@gnodet
Copy link
Contributor Author

gnodet commented Sep 11, 2024

Simple as that.

But a "minority report": I personally disagree with this, as it will encourage using A only for dependencies (as G and V are inferred), thus, will make depMgt less compelling. Not to mention how will this plus BOMs behave?

Not sure to understand. We now warn when using BOMs inside a reactor for dependency management and the only point in using dependency management for dependencies coming from the reactor is exactly to avoid writing the version. I don't really see how this is different. Note that this only applies to dependencies that are part of the reactor. Projects that have multiple subprojects usually define in their dependency management section the list of artifacts actually produced during the build to avoid having to specify the version everywhere. That's what we have in maven, maven-resolver and almost all projects I've seen. I don't see any benefit in having to maintain this section manually, since there's absolutely no value there. What am I missing ? @cstamas

@gnodet
Copy link
Contributor Author

gnodet commented Sep 11, 2024

Simple as that.
But a "minority report": I personally disagree with this, as it will encourage using A only for dependencies (as G and V are inferred), thus, will make depMgt less compelling. Not to mention how will this plus BOMs behave?

Not sure to understand. We now warn when using BOMs inside a reactor for dependency management and the only point in using dependency management for dependencies coming from the reactor is exactly to avoid writing the version. I don't really see how this is different. Note that this only applies to dependencies that are part of the reactor. Projects that have multiple subprojects usually define in their dependency management section the list of artifacts actually produced during the build to avoid having to specify the version everywhere. That's what we have in maven, maven-resolver and almost all projects I've seen. I don't see any benefit in having to maintain this section manually, since there's absolutely no value there. What am I missing ? @cstamas

I've just added location tracking. Would you like to add more checks ? Instead of using the current pom groupId to infer the missing value, we could check if there's a single matching artifactId in the reactor and use that one instead, just like we do for the version.

@gnodet gnodet force-pushed the MNG-7255 branch 2 times, most recently from 402ce62 to 3c2b2c4 Compare September 12, 2024 04:41
@gnodet gnodet changed the title [MNG-7255] Infer groupId for dependencies [MNG-7255] Infer groupId for intra-reactor dependencies Sep 18, 2024
@gnodet gnodet merged commit 2f5f61a into apache:master Sep 18, 2024
13 checks passed
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