cloudfoundry-community / node-cfenv

easy access to your Cloud Foundry application environment for node
Apache License 2.0
73 stars 20 forks source link

Remove node_modules from .cfignore #8

Closed dotchev closed 8 years ago

dotchev commented 8 years ago

There are several good reasons to avoid npm install during productive use. Currently npm does not guarantee that you will get the same (bit for bit) dependencies as during your tests. So this results in the following risks:

So it should be possible to push an app with pre-downloaded dependencies, i.e. with node_modules. Currently this is not possible as .cfignore skips node_modules from cf push.

pmuellr commented 8 years ago

It's true that there are good reasons to avoid npm install. However, there are also good reasons to use npm install. Is there a way we can support both scenarios?

I think if you're going to remove node_modules from .cfignore, you prolly want to remove it from .gitignore as well.

Perhaps the best story for folks who want a customized version, is to fork the project, change whatever you want, and then in your package.json, reference cfenv via a git url.

dotchev commented 8 years ago

Some people do commit node_modules in git, others download them during build and some run npm install during deployment (e.g. cf push). See http://www.letscodejavascript.com/v3/blog/2014/03/the_npm_debacle.

In any case it should be up to the application to decide. A dependency should not force a particular process. One can easily add/remove node_modules in .cfignore and/or .gitignore at the root of their app.

pmuellr commented 8 years ago

So, looking at this again, I think you're right. Would you like to send a PR with the fixed .cfignore file?

pmuellr commented 8 years ago

@dotchev - I updated npm to a 1.0.1 with your fix. Please test and post results back here.

pmuellr commented 8 years ago

closing as I believe this has been resolved