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

eguiraud
Copy link
Contributor

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

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

Signed-off-by: Enrico Guiraud <enrico.guiraud@cern.ch>
@eguiraud
Copy link
Contributor Author

Resolved conflicts, ready to merge

@SISheogorath SISheogorath merged commit 313eb74 into hedgedoc:master Dec 20, 2019
@ccoenen
Copy link
Contributor

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.

@eguiraud
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor

ccoenen commented Dec 20, 2019

ok, got it :-)

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.

None yet

3 participants