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

Make it easier to add hosting services#146

Closed
HaoK wants to merge 11 commits intodevfrom
morservices1-21
Closed

Make it easier to add hosting services#146
HaoK wants to merge 11 commits intodevfrom
morservices1-21

Conversation

@HaoK
Copy link
Member

@HaoK HaoK commented Jan 22, 2015

Fixes #145

@HaoK
Copy link
Member Author

HaoK commented Jan 22, 2015

Hosting.Create() basically is the new fallback service provider method with this change...

/cc @davidfowl @lodejard @Tratcher

@HaoK
Copy link
Member Author

HaoK commented Jan 22, 2015

Question, do we want to allow the additional services to override the fallback ones? Currently we add the fallback services after the additional, so I'd just have to flip the order we add the services when Importing if we want to enable overriding behavior...

@davidfowl
Copy link
Member

@HaoK what about making this Action<IServiceCollection> ?

Copy link
Member Author

Choose a reason for hiding this comment

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

@davidfowl The code here, is passing the collection to the manifest

The complication with making it an Action, is that we need to be able to figure out what services were added so we add them to the manfiest. If we take the ServiceCollection itself rather than the action, we know that every service in there should be part of the manivest. If we simply run the action as part of the service collection we are building, we have no way to determine which services came from hosting (but shouldn't be exported), vs ones they really want added.

@HaoK
Copy link
Member Author

HaoK commented Feb 6, 2015

Updated with Action

@davidfowl
Copy link
Member

This makes me wonder if things would be simpler if we just queried for IEnumerable in the Import method instead.

@HaoK
Copy link
Member Author

HaoK commented Feb 9, 2015

Good call on moving the bookkeeping to Import, makes the code quite compact now.

Also found a few minor issues (missing null guard in ConfigureHostingEnvironment, and fixed an issue with the TestServer unit test using the wrong overload of TestServer.Create

Copy link
Member

Choose a reason for hiding this comment

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

Use named arguments when passing null.

@HaoK
Copy link
Member Author

HaoK commented Feb 18, 2015

Updated PR removing optional parameters, and moving IConfiguration last for Create

@HaoK HaoK closed this in 2ee1273 Feb 19, 2015
@HaoK HaoK reopened this Feb 20, 2015
@ghost ghost added the cla-not-required label Feb 20, 2015
@HaoK
Copy link
Member Author

HaoK commented Feb 20, 2015

Reverted since 3 MVC functionals failed from this, need to investigate

@davidfowl
Copy link
Member

:shipit: When @HaoK figures out the bug

@HaoK HaoK force-pushed the morservices1-21 branch from 5a94855 to 4b66b52 Compare March 5, 2015 20:20
HaoK added a commit to aspnet/Mvc that referenced this pull request Mar 5, 2015
- needed to unblock Hosting PR:
aspnet/Hosting#146
- Service fallback currently is broken with generic services (ILogger<>
no worky)

Hosting issue tracking: aspnet/Hosting#180
@davidfowl
Copy link
Member

@HaoK I want one more overload:

HostingServices.Create(Action<IServiceCollection> configureHostServices)

That make the total 7 !

@davidfowl
Copy link
Member

We should also look at cleaning up the API a bit. What purpose does the returned IServiceCollection serve if you can't add anything to it.

@HaoK
Copy link
Member Author

HaoK commented Mar 6, 2015

Well, we use it to create the service collection, so we pass it around like in StartupLoader, and it seems nicer to let Create new up the Collection than require all callers to pass in the service collection they want to import into. The first iteration of this looked more like that, where it was Import instead of Create and took the collection as a parameter.

@HaoK
Copy link
Member Author

HaoK commented Mar 6, 2015

Overload added 28cc37d

@davidfowl davidfowl closed this Mar 6, 2015
@HaoK HaoK deleted the morservices1-21 branch May 21, 2015 17:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants