Skip to content
This repository was archived by the owner on Nov 2, 2018. It is now read-only.

Conversation

@henkmollema
Copy link
Contributor

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*

ServiceCollectionExtensions now only consists of AddTransient, AddScoped, AddSingleton and AddInstance methods.

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

@henkmollema
Copy link
Contributor Author

@henkmollema
Copy link
Contributor Author

Not sure why test fails, runs fine locally.

@pranavkm
Copy link
Contributor

pranavkm commented Aug 5, 2015

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?

@henkmollema
Copy link
Contributor Author

Sure!

Copy link
Contributor

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.

Copy link
Contributor Author

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?

Copy link
Contributor

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 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Eilon updated.

@Eilon
Copy link
Contributor

Eilon commented Aug 5, 2015

Thanks, can you squash the commits so that we can merge?

@henkmollema henkmollema changed the title Move advanced IServiceCollection extensions to FrameworkExtensions namemespace Move advanced IServiceCollection extensions Aug 5, 2015
@henkmollema
Copy link
Contributor Author

@Eilon squashed and rebased! 😄

I'll make a list a list of repos to update.

@henkmollema
Copy link
Contributor Author

Repos which need to react:

  • Mvc
  • EntityFramework
  • Antiforgery
  • EventNotification
  • Security
  • HttpAbstractions
  • Identity
  • Caching
  • Localization
  • CORS
  • DataProtection
  • Options
  • Logging

More..?

@HaoK
Copy link
Member

HaoK commented Aug 5, 2015

I'm not in love with FrameworkExtensions as a namepace... Maybe something bland like Infrastructure is better? Or maybe just Extensions?

@henkmollema
Copy link
Contributor Author

@HaoK I renamed it to Extensions for now. As discussed with @Eilon.

@HaoK
Copy link
Member

HaoK commented Aug 5, 2015

Ah I missed that thread since it was in a outdated diff... least I suggested the same name :)

@henkmollema
Copy link
Contributor Author

Yeah 😄

First comment was still mentioning FrameworkExtensions though, corrected it now.

@halter73
Copy link
Member

halter73 commented Aug 5, 2015

It's nice to have someone doing my work for me 😄 👍

@henkmollema
Copy link
Contributor Author

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.
@henkmollema
Copy link
Contributor Author

@Eilon when are you planning to merge this? I'll have some spare time this week to create the follow-up PRs.

@pranavkm
Copy link
Contributor

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.

@henkmollema
Copy link
Contributor Author

The PRs aren't there yet. I figured we wait until this change flowed to the other components and start fixing them.

@pranavkm pranavkm closed this Aug 11, 2015
@pranavkm
Copy link
Contributor

Merged.

@henkmollema
Copy link
Contributor Author

Cool! I'll look into the follow up PRs tomorrow. 😄

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

6 participants