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

Remove .env file from .gitignore to ease installation & setup #278

Closed
emmabostian opened this issue Jan 4, 2019 · 14 comments
Closed

Remove .env file from .gitignore to ease installation & setup #278

emmabostian opened this issue Jan 4, 2019 · 14 comments
Assignees
Labels
good first issue Good for new comers

Comments

@emmabostian
Copy link
Collaborator

Currently after cloning, users have to remove the .example extension from the .env file. This is annoying. Let's just remove .env from the .gitignore.

@jagzviruz
Copy link
Member

It wouldn't be advisable to allow committing .env files all the time.
We can do a one time force commit for the file.

@trent-boyd
Copy link

I'd like to take this!

I kind of agree with @jagzviruz. .env files are usually for things that might change on a per-environment basis, but we're kind of using them as a dictionary. I don't think our social links need to be there, so could we just keep them defined in constants.js?

social: {
  // we could just remove everything before and including the ||
  FB_URL: process.env.REACT_APP_FACEBOOK_URL || 'https://www.facebook.com/codingcoachio/',
  INSTA_URL: process.env.REACT_APP_INSTA_URL || 'https://www.instagram.com/codingcoach_io/',
  TWITTER_URL: process.env.REACT_APP_TWITTER_URL || 'https://twitter.com/codingcoach_io',
  GITHUB_URL: process.env.REACT_APP_GITHUB_URL || 'https://github.com/Coding-Coach/coding-coach',
},

For the remaining variable (NODE_PATH), we could leave it defined there or prefix all our our scripts with it in package.json (e.g. "build:app": "react-scripts build" becomes something like"build:app": "cross-env NODE_PATH=src/ react-scripts build"). It's a little verbose to have it defined so many times, so I'm open to more input on that.

@crysfel
Copy link
Member

crysfel commented Jan 7, 2019

@emmawedekind I wouldn't recommend that, is only a one time thing and is well documented in the readme.

Later one we are going to have the facebook app id and other values to support oauth, those values will depend on the instance the app is running, staging, production, local.

@trent-boyd yes, we only have a couple things that are unlikely to change and probably we can just define them right in the config file.

@trent-boyd
Copy link

trent-boyd commented Jan 8, 2019

In the interest of saving some time and headaches, I think we should chat about this for a bit before we start any work.

What are the arguments against having a .env file committed, other than "it's against twelve-factor code"? I'm actually curious. From what I've seen, it's generally acceptable to use env files in create-react-app-based projects, and NODE_PATH at least saves us from having to eject and configure Webpack by hand. The rule, of course, is to still avoid committing private things like API keys.

Later one we are going to have the facebook app id and other values to support oauth, those values will depend on the instance the app is running, staging, production, local.

And those values would be configured directly on the build/deploy tool, correct?

Note that I don't have much create-react-app experience, so this is a new concept to me.

@crysfel
Copy link
Member

crysfel commented Jan 8, 2019

And those values would be configured directly on the build/deploy tool, correct?

That's correct, not sure if having the .env file uploaded to the CI will cause any issues when building. We can always try.

Note that I don't have much create-react-app experience, so this is a new concept to me.

Same here, I usually configure webpack from scratch xD

@trent-boyd
Copy link

Same here, I usually configure webpack from scratch xD

You and me both, bud. Lots of people hate it, but I kinda like it. :)

@darrenvong
Copy link

Thought I'd leave a reference to facebook/create-react-app#2403 as it seems relevant to this discussion, where Dan Abramov supports committing .env file for convenience and the fact you can't store real secrets in the client app.

@emmabostian
Copy link
Collaborator Author

@crysfel did we come to an agreement?

@crysfel crysfel added the good first issue Good for new comers label Jan 14, 2019
@crysfel
Copy link
Member

crysfel commented Jan 14, 2019

@emmawedekind no hehehehe but... in order to help people get up and running easier (In case they didn't follow the README) let's commit the .env file.

This issue is open for anyone to take, it's quite simple, just remove the .env from the .gitignore :)

@becoming-software-engineer

If trent-boyd is not taking this one, may I take?

@trent-boyd
Copy link

@becoming-software-engineer: I've got some other tickets to work, so feel free to take this. :)

@becoming-software-engineer

Thanks, @trent-boyd.
@crysfel could you please assign this issue to me?

@crysfel
Copy link
Member

crysfel commented Jan 14, 2019

@becoming-software-engineer all yours! I can't officially assign it to you until you ask access to the organization, please ask to be added in slack (#front-end-gh-access channel).

You can start working on this now, then once added to the org I will officially assign it to you.

Thanks!

@emmabostian
Copy link
Collaborator Author

I already invited you @becoming-software-engineer, you have to accept the invite :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for new comers
Projects
None yet
Development

No branches or pull requests

6 participants