search

aspnet / MetaPackages

[Archived] NuGet meta packages. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
211 stars 109 forks source link

Stop adding CommandLine configuration provider twice #243

Closed glennc closed 6 years ago

glennc commented 6 years ago

In the CreateDefaultBuilder method we appear to add the CommandLine provider twice:

  1. https://github.com/aspnet/MetaPackages/blob/fcdd25635c8174a0c8a7a0853a6f3d8620448839/src/Microsoft.AspNetCore/WebHost.cs#L173
  2. https://github.com/aspnet/MetaPackages/blob/fcdd25635c8174a0c8a7a0853a6f3d8620448839/src/Microsoft.AspNetCore/WebHost.cs#L190
Tratcher commented 6 years ago

For reference, 173 existed first. Then we added 190 so hosting/kestrel could get them. That said, do we keep this order just because we want them to override the other providers?

glennc commented 6 years ago

Ahh. We add it again so that they will overwrite any other config options even though they are also added first. Right?

JunTaoLuo commented 6 years ago

I think this was more of an oversight than a conscious decision. I think it makes more sense to remove the command line provider at 173. If the command line args are overwritten, we should preserve that for the application instead of restoring the initial values.

Tratcher commented 6 years ago

There has been much debate over the precidence of each config source. Also, changing it is potentially breaking.

glennc commented 6 years ago

Closing this. It's just jarring, not necessarily wrong by the sound of it.