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

Update Microsoft.Extensions.Options to 2.1.1 and remove duplicate functionality #4703

Closed
ReubenBond opened this issue Jun 21, 2018 · 6 comments
Assignees
Milestone

Comments

@ReubenBond
Copy link
Member

ReubenBond commented Jun 21, 2018

Now that Options 2.1.x is released, we can update to it and remove:

  • OptionBuilder<T>
  • OptionsServiceCollectionExtensions
  • OptionsBuilderConfigurationExtensions
  • ConfigureNamedOptions<TOptions, TDep>
  • PostConfigureOptions<TOptions, TDep>

Note that this would be considered a breaking change, since it removes those types (some of which are public) and modifies many configuration APIs to use the equivalent types in the options package (same type names, different namespace & assembly)

However, if we don't do it then we may see some clashes for users who update that package on their own accord (eg, since it's a dependency of ASP.NET bits).

Question is: are we ok with that minor breakage and what kind of version bump do we want to associate with it, a minor (2.1.0) or a major (3.0.0)?

PoC branch here: master...ReubenBond:update-options

@ReubenBond ReubenBond added this to the Triage milestone Jun 21, 2018
@ReubenBond ReubenBond self-assigned this Jun 21, 2018
@Vhab
Copy link
Contributor

Vhab commented Jun 22, 2018

2 cents from a user,

We'd be okay with minor breakages in 2.1.
Particularly as the local client is opening up some co-hosting opportunities, it's more likely for other (potentially clashing) asp.net core dependencies are being brought into the same service.
We're intending to start experimenting with co-hosting soon to reduce serialization roundtrips.

So our reasoning for being okay with breaking changes is, we're already intending to do some shuffling to take advantage of new things in 2.1, meaning the upgrade wouldn't be seamless for us to begin with.

@sergeybykov
Copy link
Contributor

@Vhab If we were to wait with this change until 3.0, so that we don't break people, what would be the impact on you? Using full type names with namespaces to disambiguate between the two sets of types or more than that?

@Vhab
Copy link
Contributor

Vhab commented Jun 22, 2018

I haven't specifically checked yet, but I don't think postponing this change to 3.0 would negatively impact us.
It's likely what you said it is. Might even be less when things get initialized from different source files.
It's more along the lines of, if you guys want to get this change out of the way right now, we wouldn't mind.

We host on Service Fabric, which means we tend to have to do some special massaging to bootstrap things in general.
I'll report back when we've had time to play with Local Client some, but it likely won't be for another while.

@sergeybykov
Copy link
Contributor

As a general rule, we try to stay behind latest versions of dependencies like that, so that we do not force Orleans users to upgrade when they may not be ready to, while those that do want to upgrade can. There's a tradeoff here between that goal and the potential temporary confusion caused by the duplication of types. We'd like to see first if the confusion is real and has a cognitive cost. It sounds like that's not likely to be a major issue in your case so far.

@ReubenBond
Copy link
Member Author

Moved to 2.3.0 based on our meeting today

@sergeybykov
Copy link
Contributor

Resolved via #5385.

@dotnet dotnet locked as resolved and limited conversation to collaborators Sep 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants