mozilla / id.webmaker.org

OAuth 2.0 identity provider for Webmaker
https://id.webmaker.org
Mozilla Public License 2.0
18 stars 51 forks source link

Add pre-install script to copy env before install #428

Closed ryanwarsaw closed 7 years ago

ryanwarsaw commented 7 years ago

This verifies that the .env file has been copied before trying to run the post-install script.

ryanwarsaw commented 7 years ago

cc: @gideonthomas @cadecairos R+?

Pomax commented 7 years ago

R+ on my side

gideonthomas commented 7 years ago

I'm pretty sure the pre-install script is run on heroku as well. Won't this then use what's in sample.env over what's in the actual env?

Pomax commented 7 years ago

Good point! Easy enough to fix though: this is a node script, so we can simply add an if (process.env.NODE_ENV === "production") process.exit(0);

ryanwarsaw commented 7 years ago

@Pomax @gideonthomas Thanks for catching that, i went ahead and made the review changes requested, and learned a lot about Node's process API and environment-related stuff 👀 R+?

I also wanted to pick your brains on a question related to styling and JS, I'm used to working with languages like Java where there are several prominent standards (Google & Oracle's guides). However, with JS it seems to be much more diverse and spread out across the board. Is there a prominent standard with JS, or is it one of those things where as long as it passes lint it's okay?

🤔 💭

ryanwarsaw commented 7 years ago

I just realized I have to check for staging as well, I'm going to do it in the morning since it's 1 AM and I can't think straight. 😴

Pomax commented 7 years ago

@ryanwarsaw you can just check for a NODE_ENV var at all. If there is one, don't do anything, because whoever has that set clearly has special intentions.

ryanwarsaw commented 7 years ago

@Pomax R+?

ryanwarsaw commented 7 years ago

@Pomax R+? Thanks for the reviews.

Pomax commented 7 years ago

yup R+