Skip to content

Conversation

@sschuberth
Copy link
Member

Please have a look at the individual commit messages for the details.

@sschuberth sschuberth requested a review from a team as a code owner November 7, 2025 14:22
}

addApplicableLicenseFiles(licenseFiles)
addApplicableLicenseFiles(noticeFiles)
Copy link
Member

Choose a reason for hiding this comment

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

Why should these be treated the same as license files?

(This imho should also have a rationale in the commit msg)

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

  1. what the main license of a package is (also in SPDX report
  2. what is put into the notice file

Copy link
Member

Choose a reason for hiding this comment

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

Or does this change only lead to the files being include by the file archiver, but does not have any effect otherwise?

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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?

@sschuberth sschuberth requested review from a team and fviernau November 7, 2025 15:01
This is a fixup for a71ed8b.

Signed-off-by: Sebastian Schuberth <sebastian@doubleopen.org>
@sschuberth sschuberth enabled auto-merge (rebase) November 7, 2025 16:35
@codecov
Copy link

codecov bot commented Nov 7, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 57.43%. Comparing base (0839b7b) to head (462130a).

Additional details and impacted files
@@            Coverage Diff            @@
##               main   #11059   +/-   ##
=========================================
  Coverage     57.42%   57.43%           
  Complexity     1701     1701           
=========================================
  Files           346      346           
  Lines         12835    12838    +3     
  Branches       1215     1215           
=========================================
+ Hits           7370     7373    +3     
  Misses         4998     4998           
  Partials        467      467           
Flag Coverage Δ
funTest-external-tools 13.49% <0.00%> (-0.01%) ⬇️
funTest-no-external-tools 31.08% <0.00%> (-0.02%) ⬇️
test-ubuntu-24.04 42.37% <100.00%> (+0.01%) ⬆️
test-windows-2025 42.35% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Relates to #6399.

Signed-off-by: Sebastian Schuberth <sebastian@doubleopen.org>
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.

3 participants