-
Notifications
You must be signed in to change notification settings - Fork 74
[MASSEMBLY-832] Consider dependency optional attribute when gather de...#49
[MASSEMBLY-832] Consider dependency optional attribute when gather de... #49zabetak wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if the defallt here should be true to avoid changing existing behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the default to true to retain the previous behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a boolean or a list? What if someone wants to include some optional dependencies and not others?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They could possibly achieve this by declaring a separate dependencySet. It doesn't make much sense to make this very specific cause in that case we could use the existing include/exclude options.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no logging at all please. Use a SilentLogger
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the logger as part of the rebase.
jira-importer
commented
Jun 17, 2025
Resolve #1056
2b3d6cd to
b8594a9
Compare
zabetak
commented
Jun 25, 2025
Thanks for the review @elharo ! I rebased the PR against master, addressed all review comments, and also added a new integration test for the new functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
am1 --> artifact1
am2 --> artifact2
here and elsewhere avoid abbreviations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a --> artifact
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to sue mocks here. You can simply create a DefaultArtifact.
Uh oh!
There was an error while loading. Please reload this page.
https://issues.apache.org/jira/browse/MASSEMBLY-832
Maven allows to declare dependencies as optional and this is important for various reasons (e.g., avoiding licensing problems).
This change provides a convenient way to include/exclude optional dependencies in a
dependencySet.By default optional dependencies are excluded from the
dependencySet. An optional dependency shouldn't be packaged, distributed, or transitively inherited unless specified explicitly so it makes sense to exclude them by default.Following this checklist to help us incorporate your contribution quickly and easily:
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
[MASSEMBLY-XXX] - Fixes bug in ApproximateQuantiles,where you replace
MASSEMBLY-XXXwith the appropriate JIRA issue. Best practiceis to use the JIRA issue title in the pull request title and in the first line of the
commit message.
mvn clean verifyto make sure basic checks pass. A more thorough check willbe performed on your pull request automatically.
mvn -Prun-its clean verify).If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.
To make clear that you license your contribution under the Apache License Version 2.0, January 2004 you have to acknowledge this by using the following check-box.