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

Adding option to configure services when exposing the ASP.NET 5 pipel… #468

Merged
merged 1 commit into from
Nov 17, 2015

Conversation

JunTaoLuo
Copy link
Contributor

…ine via OWIN #398

@dnfclas
Copy link

dnfclas commented Nov 9, 2015

Hi @JunTaoLuo, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

@JunTaoLuo
Copy link
Contributor Author

@Tratcher @davidfowl

serviceCollection.AddInstance(new FakeService());
});

Assert.NotNull(serviceProvider);
Copy link
Member

Choose a reason for hiding this comment

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

You need to assert that you can actually resolve the FakeService

@JunTaoLuo
Copy link
Contributor Author

🆙📅

@Tratcher
Copy link
Member

Tratcher commented Nov 9, 2015

:shipit:

@@ -65,23 +66,29 @@ public static IApplicationBuilder UseOwin(this IApplicationBuilder builder, Acti
return builder;
}

public static IApplicationBuilder UseBuilder(this AddMiddleware app)
public static IApplicationBuilder UseBuilder(this AddMiddleware app, Action<IServiceCollection> configureServices = null)
Copy link
Member

Choose a reason for hiding this comment

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

No optional params. Add 2 overloads

@JunTaoLuo JunTaoLuo force-pushed the johluo/398-owin-extensions-configure-services branch from 5ab468f to 8f4c409 Compare November 11, 2015 00:19
@JunTaoLuo
Copy link
Contributor Author

🆙📅

@@ -10,7 +10,8 @@
"keyFile": "../../tools/Key.snk"
},
"dependencies": {
"Microsoft.AspNet.Http": "1.0.0-*"
"Microsoft.AspNet.Http": "1.0.0-*",
"Microsoft.Extensions.DependencyInjection": "1.0.0-*"
Copy link
Member

Choose a reason for hiding this comment

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

Microsoft.Extensions.DependencyInjection.Abstractions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need the implementation ServiceCollection from DependencyInjection. That is not available in DependencyInject.Abstractions.

@davidfowl
Copy link
Member

Squash and :shipit:

using Microsoft.Extensions.DependencyInjection;
using Xunit;


Copy link
Contributor

Choose a reason for hiding this comment

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

Extra newline.

@pranavkm
Copy link
Contributor

:shipit: once comments are addressed.

@JunTaoLuo JunTaoLuo force-pushed the johluo/398-owin-extensions-configure-services branch 2 times, most recently from 35e25ee to 19e42a4 Compare November 17, 2015 18:59
@JunTaoLuo JunTaoLuo force-pushed the johluo/398-owin-extensions-configure-services branch from 19e42a4 to fbec068 Compare November 17, 2015 19:02
@JunTaoLuo JunTaoLuo merged commit fbec068 into dev Nov 17, 2015
@JunTaoLuo JunTaoLuo deleted the johluo/398-owin-extensions-configure-services branch November 17, 2015 23:53
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.

6 participants