-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Why recommend to commit node_modules for deployment? #622
Comments
Sometimes it is a mistake, sometimes it is not, it depends on your workflow really. |
could you describe a workflow where you need to commit node_modules and shrinkwrap would not work? |
|
what does that mean? :) |
Well it is one of the major advantages of having node_modules checked in. |
sure, but can you explain when do you use git bisect, and what problems does it solve? |
This isn't really the place to discuss this, since it has nothing to do with pm2. Here is one opinion about it that makes sense: http://www.futurealoof.com/posts/nodemodules-in-git.html It should probably be removed/changed in README though, because it's not "best practice", it's just one of possible setups. |
I think this is a legitimate issue since pm2 supports deployment now. The post you mentioned is from 2011 when npm shrinkwrap was not available. |
Embedding dependencies make your app more independent.
|
In my experience committing dependencies may work if you are the only developer on an app, as soon as there are several it becomes problematic, you really don't want to handle merge conflicts on the modules you depend on. This problems grows exponentially with the amount of developers working simultaneously... |
@Unitech good tip, maybe you could add it to the README as well? |
In our deployment process, we commit |
I think this is a big mistake, will cause more problems that it solves, why not use npm shrinkwrap instead which does the same but without needing to commit a lot of unnecessary code to git?
https://www.npmjs.org/doc/cli/npm-shrinkwrap.html
The text was updated successfully, but these errors were encountered: