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

React to GetDefaultServices -> AddXXX changes #1129

Closed
wants to merge 2 commits into from
Closed

React to GetDefaultServices -> AddXXX changes #1129

wants to merge 2 commits into from

Conversation

HaoK
Copy link
Member

@HaoK HaoK commented Nov 24, 2014

EF can probably switch to just adding required services now that AddXXX is safe to call multiple times and no-ops if already there.

Should be able to just AddOptions/AddTypeActivator/AddLogging now. I didn't make this change for EF since it wasn't immediately clear what code could be removed safely

@ajcvickers
Copy link
Contributor

So does this change the behavior for replacing an already added service? In other words, if I first do AddScoped<IFoo, DefaultFoo> and then do AddScoped<IFoo, MyCustomFoo> is the second call now a no-op instead of a last wins?

@HaoK
Copy link
Member Author

HaoK commented Nov 24, 2014

If you use the new extension method (that will go in with this wave of PRs): bool TryAdd(IServiceDescriptor), it will return false/no-op if it finds a descriptor of the Service type already added. This is what all of the AddXXX methods are supposed to use. EF should switch to that for all the services inside AddEntityFramework().

@ajcvickers
Copy link
Contributor

Cool. I'll take care of that. Signing off on this change. :shipit:

@HaoK
Copy link
Member Author

HaoK commented Nov 25, 2014

Merged 88cba14

@HaoK HaoK closed this Nov 25, 2014
ajcvickers added a commit that referenced this pull request Nov 27, 2014
…ent.

Issue #956 and also see pull request #1129.

This change updates all of our AddXxx methods to use TryAdd when adding to the service collection such that only services that are not already registered will be registered. This also means that the code we had to check the service collection for "hosting" services is no longer needed.
ajcvickers added a commit that referenced this pull request Nov 27, 2014
…ent.

Issue #956 and also see pull request #1129.

This change updates all of our AddXxx methods to use TryAdd when adding to the service collection such that only services that are not already registered will be registered. This also means that the code we had to check the service collection for "hosting" services is no longer needed.

Note that services for which it is expected that multiple instances can be registered (e.g. listeners and data sources) must not use TryAdd since otherwise only the first registered will be used.
ajcvickers added a commit that referenced this pull request Dec 2, 2014
…ent.

Issue #956 and also see pull request #1129.

This change updates all of our AddXxx methods to use TryAdd when adding to the service collection such that only services that are not already registered will be registered. This also means that the code we had to check the service collection for "hosting" services is no longer needed.

Note that services for which it is expected that multiple instances can be registered (e.g. listeners and data sources) must not use TryAdd since otherwise only the first registered will be used.
@bricelam bricelam deleted the Fix1123 branch February 11, 2015 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants