Install node dependencies

[cotsog]

No description provided.

[knod]

Someday I want to know how you dug this up! Thanks! I'd appreciate if other folks check this over too.

[ethanbb]

Firstly thanks for figuring this out, we were beating our heads against the wall about this (in hindsight we probably should've thought of looking at this file...). I think we actually don't need to install codecov globally - the documentation actually suggests installing it as a dev dependency and calling it from the node_modules/.bin folder. That seems more elegant to me (and .bin is automatically on the path for Travis builds). So can we just remove the install section altogether and install codecov with --save-dev instead?

[MichaelDimmitt]

• put the comment that was here into the review

[MichaelDimmitt]

apologies about the travis.yml not having npm install
@cotsog, thanks for the fix.
Related to #632

Here is a great article on global node_modules: link

Three options relating to @ethanbb's review:

Option 1:

we dont need to add a nodemodule just put this in the travis.yml

curl -s https://codecov.io/bash | bash

Option 2

@ethanbb, you can install it with --save-dev

A simpler solution you may consider that does not require adding a script:

I tested this and it works.

npm install codecov --save-dev

then in the travis.yml file:

node_modules/.bin/codecov

Option 3:

"NPM has a slightly hidden feature where it adds node_modules/.bin to the front of your PATH environment variable whenever npm run script is executed.

// package.json
"scripts": {
  "gulp": "gulp"
},

npm run gulp

" ~ example from that article
Let me know if these changes are confusing @cotsog, I can insert what @ethanbb is referring to as an extra commit on this pull request. or in a new request.

I recommend "Option 1" I am testing it now on my fork of the project.

[ethanbb]

OK, @MichaelDimmitt, I'd prefer just adding codecov under dev dependencies I think. Anyone else have an opinion?

And when we call codecov in the .travis.yml, I believe we can just call codecov without the path, since as you said node_modules/.bin gets added to the path.

[MichaelDimmitt]

@ethanbb, node_modules/.bin gets added to the path.
Only gets added to the path if you put it into an npm script and then
in the travis.yml add `npm run <script_name>

I changed my review a bunch but it is finally complete now.
The changes were because I was learning some of it on the fly from that article 😅.

[ethanbb]

@MichaelDimmitt but I think Travis specifically adds node_modules/.bin to the path in the setup stage. You can see on line 25 of the build export PATH=./node_modules/.bin:$PATH.

[MichaelDimmitt]

@ethanbb, nice one! I verified this is correct with travis.yml, we can remove the global dependency and install it as a dev dependency and it will work.
md-web-client#3

I agree that just using codecov under dev dependencies in the package.json is the way to go!

[MichaelDimmitt]

@ethanbb, was mentioning travis.ci by default runs npm install if the install section is not specified if the install section was specified without an npm install we are guessing it was overriding the travis default and the automatic npm install was not getting called.

This means that if we do want to add an install section back in the future for some future install step then we have to make sure we do npm install as part of it. @knod, letting you into the loop.

Myself and Ethan have both made changes, I think this is good.

The alternative would be to leave the install step in the travis.yml with the added npm install so that it will never break if someone one day wants to add an additional install step.

Ethan said I should post the alternative in case anyone wanted to chime in but I agree with him this is fine for now and I am going to look through the codecov wiki documentation for this project to see if there is a spot to put this information about if someone decides to add an install step with additional install commands.

[knod]

All good points. I propose we exclude our install override, but include a comment about the default.

[knod]

In some ways, the comment isn't super useful. The crisis is over, etc. On the other hand, it's a problem we ran into, so it's a problem that's out there. Maybe it's a security blanket, but I'm ok with that if others are too, and it is better than overriding the defaults.

[ethanbb]

Does this work for you @MichaelDimmitt?

[knod]

To share the knowledge - the problem was that because of our config, specifically our cache settings, Travis didn't refresh our packages when the config install was changed. Later, maybe when we installed a new node module, the cache was cleared and we started getting the errors.

To avoid this in the future - we can either to get rid of cache in the config or to, when we edit something related to Travis, we manually go to https://travis-ci.org/codeforboston/cliff-effects/caches and delete our current cache archives.

The first would be automatic, which is a plus, but which would probably mean a longer wait time for the tests to run. The second is more knowledge that would have to be spread around, but the tests would probably be faster. Unsurprisingly to the folks who know me, I'm in favor of the first option - getting rid of the cache entirely. We could probably get rid of the comment I added at that point, and not have to add a new comment about caches! I'd like to hear the thoughts of other folks, though, and maybe this should be an issue so that others can be involved in making the decision.