Fix #351 License override does not work for third-party dependencies with multiple licenses #374
Conversation
|
Sorry for spamming the pull request with failing integration tests. Figured out how to run the integration tests locally :) This is how the log messages look like (taken from the ISSUE-351 integration test's log file: |
|
Ye, lines 178-181 don't make sense. I'll take a look at it this weekend. |
lama0206
changed the title
| licenseMap.removeProject( project ); | ||
| // remove project only removes first occurrence of project from license -> project[] map. | ||
| List<String> removedFrom = licenseMap.removeProject( project ); | ||
| LOG.info( "Overriding license(s) for dependency [{}] with [{}], overriden license(s): [{}]", |
There was a problem hiding this comment.
not good print these information to log as info. This plugin is already much verbose.
There was a problem hiding this comment.
I'm not sure if I agree - yes, it could be considered verbose yet overriding a license can be a major issue which is why I agree with digulla's comment in general.
I'd rather print one summary line (for example, "Overriding license(s) for 123 dependencies. Enable debug log for details.") iff a license has been overridden instead of silently pushing the info down to LOG.debug entirely.
What do you think?
There was a problem hiding this comment.
After testing with multi module projects with long override list I think that log in debug level will be enough.
|
Hi do you think this will be merged? |
|
Interested in this Fix as well. Wondering if this could be prioritized and moved into the next patch. |
|
merged with master |
As discussed in #351, the current license override overrides only the first license of a multi-license artifact. The update - also as discussed in #351 - removes/overrides all licenses and logs this action on the INFO level.