- Notifications
You must be signed in to change notification settings - Fork 311
Move advanced IServiceCollection extensions #274
Conversation
| Not sure why test fails, runs fine locally. |
| Tests on Travis are currently failing because of 3rd party DI tests. Those should get fixed soon. Changes look good to me. Would you be interested in creating follow up PRs in other repos where these methods are used? |
| Sure! |
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 strongly dislike this namespace name, though I can't immediately come up with a better name. Either way, please move the file into a folder that matches the namespace name.
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.
Agree, just took the suggestion of halter73. What about just Extensions or Advanced?
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.
Maybe just Extensions for now? I don't like that either, but I dislike it less 😄
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.
@Eilon updated.
| Thanks, can you squash the commits so that we can merge? |
| @Eilon squashed and rebased! 😄 I'll make a list a list of repos to update. |
| Repos which need to react:
More..? |
| I'm not in love with FrameworkExtensions as a namepace... Maybe something bland like Infrastructure is better? Or maybe just Extensions? |
| Ah I missed that thread since it was in a outdated diff... least I suggested the same name :) |
| Yeah 😄 First comment was still mentioning |
| It's nice to have someone doing my work for me 😄 👍 |
| Rebased with dev, CI is green now 😸 |
Moved advanced IServiceCollection extensions methods used primarily by frameworks into a separate namespace: `Microsoft.Framework.DependencyInjection.Extensions`. Moved: - `Add(IServiceCollection, ServiceDescriptor)` - `Add(IServiceCollection, IEnumerable<ServiceDescriptor>)` - `TryAdd(IServiceCollection, ServiceDescriptor)` - `TryAdd(IServiceCollection, IEnumerable<ServiceDescriptor>)` - `TryAdd*` The default `ServiceCollectionExtensions` now only consists of `AddTransient`, `AddScoped`, `AddSingleton` and `AddInstance` methods.
| @Eilon when are you planning to merge this? I'll have some spare time this week to create the follow-up PRs. |
| Sorry, I didn't see the follow up changes in other repos and wasn't sure if that was ready to go. We can get this change in today and work our way through others if they aren't already good to go. |
| The PRs aren't there yet. I figured we wait until this change flowed to the other components and start fixing them. |
| Merged. |
| Cool! I'll look into the follow up PRs tomorrow. 😄 |
Moved advanced
IServiceCollectionextensions methods used primarily by frameworks into a separate namespace:Microsoft.Framework.DependencyInjection.Extensions.Moved:
Add(IServiceCollection, ServiceDescriptor)Add(IServiceCollection, IEnumerable<ServiceDescriptor>)TryAdd(IServiceCollection, ServiceDescriptor)TryAdd(IServiceCollection, IEnumerable<ServiceDescriptor>)TryAdd*ServiceCollectionExtensionsnow only consists ofAddTransient,AddScoped,AddSingletonandAddInstancemethods.I'm not sure if the methods I consider as 'advanced' are the same as you guys do. Shoot if you want me to move things back and forth.
#254