quaertym / ember-cli-dependency-checker

Ember CLI addon for checking missing node and bower dependencies before running ember commands
MIT License
40 stars 37 forks source link

Deployment instructions confusing / misleading #41

Closed Fryie closed 9 years ago

Fryie commented 9 years ago

As discussed in this issue, it seems that the deployment steps described in the README are confusing and won't work well. I'd suggest just advising to always delete the node_versions directory.

quaertym commented 9 years ago

@Fryie why do you install ember-cli globally on CI server?

Fryie commented 9 years ago

How would I be running ember version on the CI server if ember-cli is not installed? I suggest removing the ember version instruction from that specific section, it is highly confusing.

quaertym commented 9 years ago

OK, how about this?

(which ember && ember version) || (rm -rf node_modules/ bower_components/ && npm install && bower install)

I am not quite sure how ember is cached between builds.

stefanpenner commented 9 years ago

Why is the ember check needed?

quaertym commented 9 years ago

I think what @mixonic had in mind was that not installing dependencies again if they are already satisfied. ember version is the simplest command that triggers dependency check.

mixonic commented 9 years ago

Correct, the intent is to trigger the check of deps against shrinkwrap.json.

I doubt the SW checker is getting a lot of usage on npm 2.x though. Eager to try it out with 3.x

Fryie commented 9 years ago

Hm, so in case the dependencies didn't change at all and are still cached (semaphore does this by default for example) it would make the build somewhat faster...

You'd still have to run ember version from node_modules/ember-cli/bin/, though (since that's likely not in the path), so that part of the instruction will need to be changed.

I'd probably add some additional explanation along the lines of:

Some build servers cache the node_modules folder, so it is possible that your app's dependencies are already installed. In order to verify that the dependencies are not out of date you can use node_modules/ember-cli/bin/ember version, which (if Ember is installed) will check that the dependencies are correct.

And then maybe something about not needing to install a global ember-cli on the CI server and that tests can be run via npm run test.

quaertym commented 9 years ago

@Fryie which ember checks whether ember is installed or not, see my comment above.

Fryie commented 9 years ago

but why should ember be in the path on a CI server?

stefanpenner commented 9 years ago

but why should ember be in the path on a CI server?

yes, this also confuses me. it should be self contained in node_modules, so running:

npm install
npm run test:ci
quaertym commented 9 years ago

@stefanpenner So the only way to be sure that all dependencies are satisfied is to remove node_modules and reinstall with npm i? Then there will be no benefit for caching node_modules for people using npm-shrinkwrap.

Fryie commented 9 years ago

well, if the folder is cached and a version of ember is installed, then node_modules/ember-cli/bin/ember version should check the contents of the node_modules folder against the shrinkwrap - so that might actually work for caching.

quaertym commented 9 years ago

@Fryie Is this better?

([ -f node_modules/ember-cli/bin/ember ] && node_modules/ember-cli/bin/ember version) || (rm -rf node_modules/ bower_components/ && npm install && bower install)
Fryie commented 9 years ago

I think that works, at least on my CI server it does (I didn't add the [ -f node_modules/ember-cli/bin/ember ] part, but it is somewhat cleaner having it).

quaertym commented 9 years ago

Skipping that part results in No such file or directory error if node_modules does not exist.

quaertym commented 9 years ago

For further discussion: https://github.com/quaertym/ember-cli-dependency-checker/pull/42