Notice followups#11059
Conversation
| } | ||
|
|
||
| addApplicableLicenseFiles(licenseFiles) | ||
| addApplicableLicenseFiles(noticeFiles) |
There was a problem hiding this comment.
Why should these be treated the same as license files?
(This imho should also have a rationale in the commit msg)
There was a problem hiding this comment.
I believe the function name is just misleading. We use the same function for adding any files, including patentFiles and otherLicenseFiles. So it seems natural to also use it for noticeFiles, and I don't think a justification is needed.
There was a problem hiding this comment.
Hm, the function docs say "Return a mapping from the given relative [directories] to the relative paths of the (root) licenses file". So, the result is considered a "root license" file. An I suppose (haven't checked), that the returned stuff is treated like a root license file. For example when inserting original license texts into a notice. E.g. would this change here alter
- what the main license of a package is (also in SPDX report
- what is put into the notice file
There was a problem hiding this comment.
Or does this change only lead to the files being include by the file archiver, but does not have any effect otherwise?
There was a problem hiding this comment.
Or does this change only lead to the files being include by the file archiver, but does not have any effect otherwise?
That's at least exactly the intention of this change. Also see this Slack conversation.
There was a problem hiding this comment.
This is called by matchWithRootLicenses() and looking at the callers of that one, to me it is not clear why there should be no effect on those. Can you please explain?
There was a problem hiding this comment.
I've finally found the time to revisit the mainLicense() code: In essence, it creates detectedLicenses from those applicableLicenseFiles with license findings, and I agree that notice files should not be considered here, as notice files do not contain the license(s) of the project(s), but their of dependencies.
So I'll drop the last commit, @fviernau.
cedf1b7 to
00dac6f
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11059 +/- ##
============================================
- Coverage 58.60% 58.59% -0.02%
Complexity 1809 1809
============================================
Files 359 359
Lines 13387 13390 +3
Branches 1355 1355
============================================
Hits 7846 7846
- Misses 5049 5052 +3
Partials 492 492
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
00dac6f to
462130a
Compare
This is a fixup for a71ed8b. Signed-off-by: Sebastian Schuberth <sebastian@doubleopen.org>
23a45cf to
fa3e93f
Compare
Please have a look at the individual commit messages for the details.