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

Pass in --environment production to denali publish #397

Closed
wants to merge 1 commit into from

Conversation

seawatts
Copy link
Contributor

@davewasmer
Copy link
Collaborator

I think we actually want to use an environment variable. This -- syntax with npm run means the remaining arguments will be passed to the script that npm run build executes, but we can't guarantee that will be denali build - someone may introduce their own build/prebuild script that invokes denali build along the way. But if we set an environment variable, that has a much better chance of surviving through whatever build script the user adds and making it to the denali build process.

@seawatts
Copy link
Contributor Author

seawatts commented Oct 5, 2017

Will denali build in production if we set NODE_ENV=production?

@davewasmer
Copy link
Collaborator

It should, although I'm not 100% sure it does.

@davewasmer davewasmer added this to the v0.1.0-beta.1 milestone Oct 24, 2017
@davewasmer
Copy link
Collaborator

Closing for lack of activity.

This is something we want to do though (build for production on publish). But see the comments above: environment variable is likely the best way to accomplish this.

If anyone is interested in carrying this across the finish line: we need to verify that NODE_ENV actually does set the environment during build, and then set it here.

@davewasmer davewasmer closed this Jan 31, 2018
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.

2 participants