-
Notifications
You must be signed in to change notification settings - Fork 162
process.env #81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
@DanielRuf Thanks for pointing it out. I added .env files with the variables as part of #57, and I think I missed to commit it. |
Ahh! Now I see why I missed to add .env file. @crysfel, @emmawedekind I think we can remove .env file from .gitignore. It makes sense to ignore .env.local or.env.* but not .env file. If the developer has to change the value of .env file, hr/she can create .env.local file and make changes. Thoughts? |
I think that's fine! I have approved the PR. |
When I started the Google Analytics PR #70, I also included setting up |
I'm wondering if |
I don't think we should have a https://www.npmjs.com/package/dotenv#should-i-commit-my-env-file Is there a reason why those links are set using environment variables (slack etc.)? I don't think those should be set by the environment. If anything, you could set default values if the environment variables aren't detected. We can have a sample Just my two cents! 🤷♂️ |
Exactly. dot-env are normally meant for credentials and user specific settings. |
Issue
Description
Using
process.env
is not great as the used env variables seem to be not declared anywhere. We should usecross-env
for testing purposes and use some config for setting them instead of usingprocess.env
.The text was updated successfully, but these errors were encountered: