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

Add extension method for getting environment name #194

Merged
merged 1 commit into from
Mar 17, 2015
Merged

Add extension method for getting environment name #194

merged 1 commit into from
Mar 17, 2015

Conversation

Praburaj
Copy link
Contributor

@ghost ghost added the cla-not-required label Mar 16, 2015
@muratg
Copy link

muratg commented Mar 17, 2015

:shipit:

@Praburaj Praburaj merged commit 3b0d5fd into aspnet:dev Mar 17, 2015
/// <returns>True if the specified name is same as the current environment.</returns>
public static bool IsEnvironment(
[NotNull]this IHostingEnvironment hostingEnvironment,
[NotNull]string environmentName)
Copy link
Member

Choose a reason for hiding this comment

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

Should this really throw on null?

Copy link
Member

Choose a reason for hiding this comment

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

(string.Equals() doesn't care about null, so the rest of the code would still work just fine)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is null a valid environment name? I think there is always a default in hosting layer. That's why I made it throw on null.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I'm thinking more of a mock/lazy implementation of the interface that happens to return null. Would we want that to fail?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm. I think I can remove the null check on environmentName.

kirthik added a commit that referenced this pull request May 22, 2015
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.

3 participants