Skip to content
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

Fix Configuration.Binder to bind to IList<T> properties #73267

Merged
merged 1 commit into from
Aug 3, 2022

Conversation

eerhardt
Copy link
Member

@eerhardt eerhardt commented Aug 2, 2022

Binding to an IList in ConfigurationBinder was failing because we were only checking for ICollection. The fix is to also check for IList.

Fix #73221

cc @SteveDunn

Binding to an IList<T> in ConfigurationBinder was failing because we were only checking for ICollection<T>. The fix is to also check for IList<T>.

Fix dotnet#73221
@ghost
Copy link

ghost commented Aug 2, 2022

Tagging subscribers to this area: @dotnet/area-extensions-configuration
See info in area-owners.md if you want to be subscribed.

Issue Details

Binding to an IList in ConfigurationBinder was failing because we were only checking for ICollection. The fix is to also check for IList.

Fix #73221

cc @SteveDunn

Author: eerhardt
Assignees: -
Labels:

area-Extensions-Configuration

Milestone: -

@@ -282,33 +282,6 @@ private void GetIntDictionaryT<T>(T k1, T k2, T k3)
Assert.Equal("val_2", options[k3]);
}

[Fact]
Copy link
Member Author

Choose a reason for hiding this comment

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

These two tests are 100% duplicates with the BindXXX tests directly below them. No reason to have duplicate tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for fixing so quickly

Copy link
Contributor

@SteveDunn SteveDunn left a comment

Choose a reason for hiding this comment

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

Oops, sorry about causing a regression!

@eerhardt eerhardt merged commit 3b3645e into dotnet:main Aug 3, 2022
@eerhardt eerhardt deleted the Fix73221 branch August 3, 2022 14:47
@ghost ghost locked as resolved and limited conversation to collaborators Sep 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants