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

Breaking change in ResourceSet class #44483

Closed
sebastienros opened this issue Nov 10, 2020 · 8 comments · Fixed by #44482
Closed

Breaking change in ResourceSet class #44483

sebastienros opened this issue Nov 10, 2020 · 8 comments · Fixed by #44482

Comments

@sebastienros
Copy link
Member

This PR #44104 introduces a breaking change that is only experienced at runtime.
ResourceSet implements IEnumerable and ASP.NET Core is using the contract.

At least there, but other tests break so it might not be isolated.
https://github.com/dotnet/aspnetcore/blob/master/src/Localization/Localization/src/Internal/ResourceManagerStringProvider.cs#L75

While we could react to this change, it doesn't seem that this change of behavior was intentional.

@jkotas
Copy link
Member

jkotas commented Nov 10, 2020

Revert in #44482

@ghost
Copy link

ghost commented Nov 10, 2020

Tagging subscribers to this area: @tarekgh, @buyaa-n, @krwq, @jeffhandley
See info in area-owners.md if you want to be subscribed.


Issue meta data

Issue content: This PR https://github.com//pull/44104 introduces a breaking change that is only experienced at runtime. `ResourceSet` implements `IEnumerable` and ASP.NET Core is using the contract.

At least there, but other tests break so it might not be isolated.
https://github.com/dotnet/aspnetcore/blob/master/src/Localization/Localization/src/Internal/ResourceManagerStringProvider.cs#L75

While we could react to this change, it doesn't seem that this change of behavior was intentional.

Issue author: sebastienros
Assignees: -
Milestone: -

@marek-safar
Copy link
Contributor

@sebastienros what is the failure?

@sebastienros
Copy link
Member Author

I left all the details on the PR. To summarize ResourceSet implements IEnumerable but the returned elements are not of the same type. They were DictionaryEntry and now they are KeyValuePair<>.

@tarekgh
Copy link
Member

tarekgh commented Nov 10, 2020

@marek-safar here is @sebastienros comment regarding the failure #44104 (comment)

@marek-safar
Copy link
Contributor

I see you are casting object to underlying type. That needs to be fixed on runtime side.

@tarekgh
Copy link
Member

tarekgh commented Nov 10, 2020

@marek-safar just to make sure you saw @jkotas PR reverting your change which broke this scenario #44482

@marek-safar
Copy link
Contributor

Yep, let's revert this to unblock them. I'll fix that in follow up PR (revert)

@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants