-
Notifications
You must be signed in to change notification settings - Fork 311
Create IHostingEnvironment. #73
Conversation
{ | ||
public class HostingEnvironment : IHostingEnvironment | ||
{ | ||
public HostingEnvironment() |
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.
Needed? (Don't really care either way.)
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.
Yea nuke this
Updated. |
1 similar comment
Updated. |
try | ||
{ | ||
var config = new Configuration(); | ||
config.AddJsonFile("project.json"); |
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.
Path.Combine(applicationBasePath, "project.json")
Updated. |
var env = serviceProvider.GetService<IApplicationEnvironment>(); | ||
if (env == null) | ||
var appEnv = serviceProvider.GetService<IApplicationEnvironment>(); | ||
if (appEnv == null) |
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.
Wondering - can the service be null? I thought DI throws if it's not registered. And that you have to ask for IEnumerable<T>
to get 0 or more instances. Do we have a unit test for these scenarios?
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.
It can't be null
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.
You're right, DI does throw now, I'll remove this old code.
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.
And add a test or two please!
After those changes |
me too, thanks! |
#71