-
Notifications
You must be signed in to change notification settings - Fork 377
Add support for required keyword #10021
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
Also avoid emitting the new `CompilerFeatureRequiredAttribute`
Use the one that the project is compiling with. This is a copy of the logic from https://github.com/dotnet/sdk/blob/d670c1d9c72190cdf8228ed000847f9bde55ce71/src/Compatibility/Microsoft.DotNet.Compatibility/ValidatePackage.cs#L47
ViktorHofer
left a comment
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.
This looks great to me. Approving, assuming that you tested this for dotnet/runtime locally.
|
I did test locally but will push up a draft PR to test in dotnet/runtime across the larger CI matrix to make sure nothing funny is going on. |
src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.Attributes.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.Attributes.cs
Show resolved
Hide resolved
src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.Attributes.cs
Outdated
Show resolved
Hide resolved
|
Here's the test PR: dotnet/runtime#72233 |
333fred
left a comment
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.
Generation changes LGTM. I did not review the other changes.
Best reviewed commit-by-commit.
This adds support for the
requiredkeyword into GenApi and then also makesGenFacadeshonorLangVersionand use the compiler version that the project is already using (similar to what we do for new API Compat).This should fix dotnet/runtime#71559