-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fix package prune data for .NET Core 2.x #50370
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.
Pull Request Overview
This PR fixes package prune data for .NET Core 2.x by enabling package pruning for netcoreapp2.0 and netcoreapp2.1 target frameworks. The fix removes the framework version condition that was preventing pruning for these versions and updates the framework package definitions to exclude the Microsoft.NETCore.App package from pruning data.
- Enables package pruning for netcoreapp2.0 and netcoreapp2.1 by removing version-based conditions
- Removes Microsoft.NETCore.App package entries from netcoreapp2.0 and netcoreapp2.1 framework package definitions
- Updates test expectations to reflect that pruning now works for these frameworks
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Microsoft.NET.Sdk.FrameworkReferenceResolution.targets | Removes framework version condition that prevented pruning for .NET Core 2.x |
| FrameworkPackages.netcoreapp2.0.cs | Removes Microsoft.NETCore.App package entry from netcoreapp2.0 framework packages |
| FrameworkPackages.netcoreapp2.1.cs | Removes Microsoft.NETCore.App package entry from netcoreapp2.1 framework packages |
| GivenThatWeWantToResolveConflicts.cs | Updates test data to expect pruning to work for netcoreapp2.0 and netcoreapp2.1 |
| RulesMissingDocumentation.md | Removes analyzer rule entries that are no longer missing documentation |
| CA1873 | <https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1873> | Avoid potentially expensive logging | | ||
| CA1874 | <https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1874> | Use 'Regex.IsMatch' | | ||
| CA1875 | <https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1875> | Use 'Regex.Count' | | ||
| CA2023 | <https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2023> | Invalid braces in message template | |
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.
@ViktorHofer We're seeing this file churn when we build locally, is this a change that should be checked in?
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 got the same thing too on my machine, so definitely some line endings that need fixed up.
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.
Yes I think this needs to get checked-in.
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.
@dsplaisted Was the auto-generation removing these lines? That doesn't look correct to me because at least CA2023 is indeed missing documentation. See #51305
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.
All these rules are actually missing documentation.
| CA1873 | <https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1873> | Avoid potentially expensive logging | | ||
| CA1874 | <https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1874> | Use 'Regex.IsMatch' | | ||
| CA1875 | <https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1875> | Use 'Regex.Count' | | ||
| CA2023 | <https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2023> | Invalid braces in message template | |
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 got the same thing too on my machine, so definitely some line endings that need fixed up.
| [InlineData("netcoreapp2.1", true)] | ||
| [InlineData("netcoreapp2.0", true)] |
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.
| [InlineData("netcoreapp2.1", true)] | |
| [InlineData("netcoreapp2.0", true)] | |
| [InlineData("netcoreapp2.1")] | |
| [InlineData("netcoreapp2.0")] |
d75e2d2 to
108de0e
Compare
Fixes #49917