-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
[Group 4] Enable nullable annotations for Microsoft.Extensions.Configuration.FileExtensions
#57434
[Group 4] Enable nullable annotations for Microsoft.Extensions.Configuration.FileExtensions
#57434
Conversation
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
Tagging subscribers to this area: @maryamariyan, @safern Issue DetailsAnnotated according to:
|
Microsoft.Extensions.Configuration.FileExtensions
# Conflicts: # src/libraries/Microsoft.Extensions.Configuration.Abstractions/ref/Microsoft.Extensions.Configuration.Abstractions.csproj # src/libraries/Microsoft.Extensions.Configuration.Abstractions/src/Microsoft.Extensions.Configuration.Abstractions.csproj # src/libraries/Microsoft.Extensions.Configuration.FileExtensions/ref/Microsoft.Extensions.Configuration.FileExtensions.csproj # src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/Microsoft.Extensions.Configuration.FileExtensions.csproj # src/libraries/Microsoft.Extensions.Configuration/ref/Microsoft.Extensions.Configuration.csproj # src/libraries/Microsoft.Extensions.Configuration/src/Microsoft.Extensions.Configuration.csproj
...nsions.Configuration.FileExtensions/ref/Microsoft.Extensions.Configuration.FileExtensions.cs
Show resolved
Hide resolved
...libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileConfigurationProvider.cs
Show resolved
Hide resolved
...nsions.Configuration.FileExtensions/ref/Microsoft.Extensions.Configuration.FileExtensions.cs
Show resolved
Hide resolved
@maxkoshevoi - Do you think you can make the same changes here as I did in #57433?
We may need to wait for #57433 to be merged in order to get the changes to MS.Ext.Configuration I made. Or you can make them now, and we can merge any conflicts once the first PR goes in. |
...nsions.Configuration.FileExtensions/ref/Microsoft.Extensions.Configuration.FileExtensions.cs
Outdated
Show resolved
Hide resolved
@maxkoshevoi - the build is broken. Can you fix it? Look to see how I did it in 99dae39 |
This reverts commit 2ed875d.
src/libraries/Microsoft.Extensions.FileProviders.Abstractions/src/NotFoundFileInfo.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/FileLoadExceptionContext.cs
Outdated
Show resolved
Hide resolved
/azp run |
Commenter does not have sufficient privileges for PR 57434 in repo dotnet/runtime |
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.
The one open question is the only comment I have left. After that is resolved, I believe this can be merged.
...nsions.Configuration.FileExtensions/ref/Microsoft.Extensions.Configuration.FileExtensions.cs
Outdated
Show resolved
Hide resolved
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 think this change looks good. Thanks for all the work here @maxkoshevoi!
@halter73 - any more thoughts?
Related to #43605
Annotated according to:
Microsoft.Extensions.Configuration
#57414Microsoft.Extensions.FileProviders.Physical
#57409