-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Include third-party-notices.txt in all packages #20166
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
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.
Looks good but let's verify the build artifacts when available.
Will do. I took a look at the artifacts on my box & they looked good, but will re-verify w/ the stuff from CI |
How will we keep the Identity/UI and Components files up-to-date @mkArtakMSFT (especially after this change gets into 'master')❔ And, is there a way to automatically verify the files are complete close to release time? |
The packages here look correct to me. @safern @ericstj @ViktorHofer does dotnet/runtime do any package testing to validate that files like |
We do some package testing but we don't have tests around those additional files, however we do include them in the dotnet/runtime packages. It wouldn't be difficult to add such a test to our validation. |
Actually, I see we have in our nuget packages, but not in the shared framework. @dagood @NikolaMilosavljevic we might want to fix that. |
The goal in the past has seemed to be that "C:\Program Files\dotnet\ThirdPartyNotices.txt" contains data for everything in the directory: dotnet/runtime#3619. |
Resolves #20086
Explicitly includes
Third-Party-Notices.txt
in all packages, and allows packages to use a customthird-party-notices.txt
file if they so choose (Identity.UI and the Components packages currently do so). Also includes theversion.txt
file that was missing from the shared framework