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

[DOC] Use `npm start`, not `yarn start` to start #239

Merged
merged 1 commit into from Dec 20, 2019

Conversation

@bluehood
Copy link
Contributor

bluehood commented Dec 15, 2019

yarn start --production ignores the --production flag,
npm start --production does not.

@bluehood bluehood force-pushed the bluehood:npm_not_yarn branch from dc3af3c to 03acfa4 Dec 17, 2019
@SISheogorath SISheogorath added this to the Release 1.6.0 milestone Dec 20, 2019
`yarn start --production` ignores the `--production` flag,
`npm start --production` does not.

Signed-off-by: Enrico Guiraud <enrico.guiraud@cern.ch>
@bluehood bluehood force-pushed the bluehood:npm_not_yarn branch from 03acfa4 to ed2a792 Dec 20, 2019
@bluehood

This comment has been minimized.

Copy link
Contributor Author

bluehood commented Dec 20, 2019

Resolved conflicts, ready to merge

@SISheogorath SISheogorath merged commit 313eb74 into codimd:master Dec 20, 2019
4 checks passed
4 checks passed
DCO DCO
Details
WIP Ready for review
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
security/snyk - package.json (codimd) No manifest changes detected
@ccoenen

This comment has been minimized.

Copy link
Member

ccoenen commented Dec 20, 2019

I know this was already merged, but shouldn't we also update

https://github.com/codimd/server/blob/313eb74ed697c5260df9293b1aefe3695cf65a5a/bin/setup

at the same time? If we stop using yarn (which I'm perfectly fine with!) we should also remove it from the setup script.

@bluehood

This comment has been minimized.

Copy link
Contributor Author

bluehood commented Dec 20, 2019

I can't comment on dropping yarn altogether. There are still calls to yarn in the manual setup instructions. The PR only changed yarn start to npm start because otherwise the --production flag is ignored.

This might very well be a bug in yarn that they will fix in a next release, I don't know.

@SISheogorath

This comment has been minimized.

Copy link
Member

SISheogorath commented Dec 20, 2019

I don't think we want to remove yarn, but just use npm for for the production start (since in yarn it simply doesn't work as with npm). When recently someone went through all files and replaced npm with yarn, it was unnoticed that npm start --production is fundamentally different from yarn start --production.

@ccoenen

This comment has been minimized.

Copy link
Member

ccoenen commented Dec 20, 2019

ok, got it :-)

@bluehood bluehood deleted the bluehood:npm_not_yarn branch Jan 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.