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

Configure ApplicationName of ApplicationEnvironment on WebHostBuilder #625

Merged
merged 1 commit into from
Mar 10, 2016

Conversation

JunTaoLuo
Copy link
Contributor

@@ -32,6 +33,8 @@ public WebHostOptions(IConfiguration configuration)

public string Application { get; set; }

public string EnvironmentApplication { get; set; }
Copy link
Member

Choose a reason for hiding this comment

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

👎 naming.

@JunTaoLuo JunTaoLuo force-pushed the johluo/configure-applicationname branch from 874efec to 10c41ff Compare March 8, 2016 19:01
appEnv = new WrappedApplicationEnvironment(_options.ApplicationBasePath, appEnv);
}
var applicationBasePath = ResolvePath(_options.ApplicationBasePath, appEnv.ApplicationBasePath);
var environmentApplicationName = ResolveStartupAssemblyName() ?? appEnv.ApplicationName;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think there will ever be a case where we default to appEnv.ApplicationName since we need to specify a startup assembly/type/method to build the webhost. But I guess keeping the fallback doesn't hurt.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah this would work for us. Is @davidfowl ok with this behavior?

Copy link
Member

Choose a reason for hiding this comment

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

Yep!

@JunTaoLuo
Copy link
Contributor Author

🆙📅

@Tratcher
Copy link
Member

Tratcher commented Mar 8, 2016

Better. @pranavkm, is that what you were after?

@@ -221,12 +221,29 @@ private IServiceCollection BuildHostingServices()
return services;
}

private string ResolveStartupAssemblyName()
{
if (_startup != null)
Copy link
Member

Choose a reason for hiding this comment

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

Add some comments here

@davidfowl
Copy link
Member

There are some other changes I'd like to see after this change is in.

@davidfowl
Copy link
Member

Why is travis failing /cc @pranavkm

@JunTaoLuo JunTaoLuo force-pushed the johluo/configure-applicationname branch from 52050d5 to b52bf96 Compare March 10, 2016 18:41
@JunTaoLuo
Copy link
Contributor Author

🆙📅 cc @Tratcher @davidfowl

if (_startup != null)
{
var startupAssemblyLocation = Path.GetDirectoryName(_startup.ConfigureDelegate.Target.GetType().GetTypeInfo().Assembly.Location);
if (startupAssemblyLocation != null) {
Copy link
Member

Choose a reason for hiding this comment

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

bad formatting 😄

_startup.ConfigureDelegate.Target.GetType().GetTypeInfo().Assembly.Location

This will be null.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's fine, Path.GetDirectoryName returns null if the path is null

@davidfowl
Copy link
Member

:shipit:

@JunTaoLuo JunTaoLuo force-pushed the johluo/configure-applicationname branch from dcd2035 to b48b5f1 Compare March 10, 2016 23:45
@JunTaoLuo JunTaoLuo merged commit b48b5f1 into dev Mar 10, 2016
@JunTaoLuo JunTaoLuo deleted the johluo/configure-applicationname branch March 10, 2016 23:46
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.

5 participants