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

Improve the comment for the prefix parameter in the AddEnvironmentVariables method #488

Closed
miguelcrpinto opened this issue Aug 16, 2016 · 5 comments
Assignees

Comments

@miguelcrpinto
Copy link
Contributor

miguelcrpinto commented Aug 16, 2016

The /// comment for the prefix parameter of the AddEnvironmentVariables method is a bit misleading as it doesn't only filter the environment variables but also removes the provided prefix from the variable name. This behavior should be mentioned in the comments.

This has already been changed/improved in the documentation side and I think it would also make sense to improve on the code comments side. Please check the pull request in the documentation repo: dotnet/AspNetCore.Docs#1777

I spent a few hours myself trying to figure out why my environment variables weren't being loaded and why weren't they overriding my variables in the appsettings.json. Lets help other developers with this small change in the comments.

@divega
Copy link

divega commented Aug 17, 2016

@miguelcrpinto are you planning to send a PR for the product to improve this? 😄

@divega divega added the bug label Aug 17, 2016
@divega divega added this to the 1.1.0 milestone Aug 17, 2016
@miguelcrpinto
Copy link
Contributor Author

@divega I could do the PR but as far as I know I'd have to sign the Contributor License Agreement which I haven't done yet and today I'm going on a short vacation until the end of the month.

I'll start by signing the Contributor License Agreement and this gets approved today I'll try to do the PR before going on vacations.

@divega
Copy link

divega commented Aug 18, 2016

@miguelcrpinto no worries and enjoy your vacation. If you don't get to it we will apply the change here.

miguelcrpinto added a commit to miguelcrpinto/Configuration that referenced this issue Aug 18, 2016
Improved the comment for the prefix parameter in the AddEnvironmentVariables method

Issue aspnet#488
@miguelcrpinto
Copy link
Contributor Author

Done! PR #492

HaoK pushed a commit that referenced this issue Aug 19, 2016
Improved the comment for the prefix parameter in the AddEnvironmentVariables method

Issue #488
@HaoK
Copy link
Member

HaoK commented Aug 19, 2016

Merged thanks! e82c7aa

@HaoK HaoK closed this as completed Aug 19, 2016
@HaoK HaoK added the 3 - Done label Aug 19, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants