Skip to content
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

Update instruction for loading dotenv #452

Merged
merged 1 commit into from
Jun 25, 2022
Merged

Conversation

ahangarha
Copy link
Contributor

Loading dotenv in production raise error for the obvious reason that we install it only for development and test environment. So I think this instruction should be included in the readme file so that we load it only if we are in the desired environments.

Loading dotenv in production raise error for the obvious reason that we install it only for development and test environment. So I think this instruction should be included in the readme file so that we load it only if we are in the desired environments.
@domcermak
Copy link

domcermak commented Jun 25, 2022

This sounds like your personal use case. It is common to use dotenv files across environments. So why change the instructions also for other devs?

@bkeepers
Copy link
Owner

Thanks for the pull request, @ahangarha. Since people can copy and paste the example and modify it to suit their needs, I think this is fine.

@bkeepers bkeepers merged commit a28205d into bkeepers:master Jun 25, 2022
@ahangarha
Copy link
Contributor Author

This sounds like your personal use case. It is common to use dotenv files across environments. So why change the instructions also for other devs?

I kind of agree with you. I think we need more robust documentation about this matter. I tried to put a comment above the line to indicate the reason we have that condition. So if someone is copy/pasting and doesn't have deeper knowledge, things go fine. Otherwise it is still easy to customize it as per the user's need.

So while I don't consider my PR as the best possible option, but I think it is one small step ahead.

Thanks for developing and maintaining this project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants