For RC2 use both environment names for dev/release etc.

[blowdart]

#720

@Eilon

In changing the name of the environment variables we've caused problems for folks that haven't switched them, so someone running RC2 code but still using the RC1 environment variable names are going to end up using production settings, which can have unintended side effects.

For RC2 we should look for both names, if the RC2 name isn't present, but the RC1 name is, we should honour that. For RTM this would be removed.

[Eilon]

Seems reasonable to me. Can you send email to me and @DamianEdwards to approve for RC2?

@muratg @davidfowl FYI.

[Eilon]

If possible, we should log something to ILogger if the app is using an old name.

[Tratcher]

ASPNET_ENV?

[blowdart]

Whatever it was before, which is different to now. I'm not a details person :D

[blowdart]

Oh there we go,

So check Hosting:Environment (or Hosting__Environment), if not present fall back to, yes, ASPNET_ENV

[Tratcher]

In RC1 we supported ASPNET_ENV as legacy and Hosting:Environment as primary.
https://github.com/aspnet/Hosting/blob/1.0.0-rc1/src/Microsoft.AspNet.Hosting/HostingEnvironmentExtensions.cs#L12-L13

Now it looks like we'd rotate these to Hosting:Environment as legacy and ASPNETCORE_ENVIRONMENT as primary.

This also relates to my other PR for when to read them. I'll just add on to that.
#735

[cwe1ss]

Thanks for considering this! I think this is all about making sure, everyone who's affected knows about this change. If you keep the old values for RC2 but remove them for RTM then people will just run into this issue later.

Since this is a hidden breaking change (different behavior at runtime) and since logging is not easy at that point, how about making this a breaking change that actually fails? For RC2, you could just throw an exception on startup if someone is still using the old keys. For RTM you could remove this hack of course.

That's not a very elegant solution but it will do the job for everyone who's following RC-bits. I believe you REALLY don't want to target the wrong environment, so such a drastic solution might be acceptable for people?!?

If you don't want to go this way, how about a new announcement that makes this issue clearer? Right now the information regarding environment variables and so on is split over multiple announcements which makes it hard to understand.

[davidfowl]

I personally don't see why we'd do this from RC1 to RC2 then remove it for RTM...

[blowdart]

@davidfowl You think keep it for RTM? I don't see why not.

[davidfowl]

Why do we need to keep it in the first place? You have to recompile your application anyways. This is just one more change you need to make. Maybe the problem is that it's not obvious what you're running and we should fix that instead.

[cwe1ss]

+1 for making the environment blink in the console output :-)

No, I also think either support both for rc2 and rtm or make this change clearer in the docs. Aren't you working on a RC2 migration guide anyway?

[Tratcher]

For RC2 I should just add the warnings for old variables, not not actually read the values. It's better to break people now than RTM.

[davidfowl]

@Tratcher I like that!

[Tratcher]

Unfortionately that's not what we ended up doing: #735 (comment)