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

Don't erase NODE_ENV from environment. #12028

Merged
merged 1 commit into from Jun 30, 2016

Conversation

@ggreer
Copy link
Contributor

commented Jun 22, 2016

Atom sets NODE_ENV to 'production' earlier, but some code paths can cause it to be un-set. This degrades performance and sometimes crashes React. Fixes #12024.

I've tested this on Linux and macOS, but not Windows.

Don't erase NODE_ENV from environment. Atom sets this to 'production'…
… earlier, but some code paths can cause it to be un-set. This degrades performance and sometimes crashes React. Fixes #12024.

@ggreer ggreer force-pushed the Floobits:node_env_fix branch from 49ee71e to 76e59a2 Jun 23, 2016

@ggreer

This comment has been minimized.

Copy link
Contributor Author

commented Jun 23, 2016

Whoops. Didn't see the style rule about single quotes. Force-pushed a fix.

@ggreer

This comment has been minimized.

Copy link
Contributor Author

commented Jun 23, 2016

The build error seems to be a failure to download a dependency.

@50Wliu

This comment has been minimized.

Copy link
Member

commented Jun 23, 2016

The failing spec has been notoriously flaky for a while now on Travis and can be safely ignored.

/cc @lee-dohm

@lee-dohm

This comment has been minimized.

Copy link
Member

commented Jun 27, 2016

I have this on my plate to look at. I'll get to it as soon as I can.

@lee-dohm

This comment has been minimized.

Copy link
Member

commented Jun 30, 2016

I got this all tested. It looks good. Thanks for contributing!

@lee-dohm lee-dohm merged commit 04ff1b5 into atom:master Jun 30, 2016

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.