Skip to content
This repository has been archived by the owner on Dec 4, 2018. It is now read-only.

Use nvm explicitly to enforce npm version #847

Merged
merged 3 commits into from
Jan 4, 2018

Conversation

chosak
Copy link
Member

@chosak chosak commented Jan 2, 2018

This change modifies how the Node version is specified in the Travis CI build. The "nodejs: 6" entry in .travis.yml wasn't properly setting the Node version to the desired 6.x version. This was causing problems because Travis was using Node 8.x, which modified the npm-shrinkwrap.json file. This in turn created a "dirty" git state which caused release wheels to be named improperly.

Ping @higs4281 who I know was experiencing something similar in another repository.

This repository should probably be upgraded to explicitly use Node 8 instead of Node 6, but that is left for a future effort.

This change modifies how the Node version is specified in the Travis CI
build. The "nodejs: 6" entry in .travis.yml wasn't properly setting the
Node version to the desired 6.x version. This was causing problems
because Travis was using Node 8.x, which modified the
npm-shrinkwrap.json file. This in turn created a "dirty" git state which
caused release wheels to be named improperly.

Ping @higs4281 who I know was experiencing something similar in another
repository.

This repository should probably be upgraded to explicitly use Node 8
instead of Node 6, but that is left for a future effort.
@higs4281
Copy link
Member

higs4281 commented Jan 2, 2018

experiencing something similar

Turned out to be a local issue.

@chosak chosak requested a review from anselmbradford January 4, 2018 14:18
Copy link
Member

@anselmbradford anselmbradford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks fine. If it passes Travis in this PR that is all that needs testing, correct?

@chosak
Copy link
Member Author

chosak commented Jan 4, 2018

@anselmbradford yes, thanks!

@chosak chosak merged commit 74eebf1 into cfpb:master Jan 4, 2018
@chosak chosak deleted the use-nvm-explicitly branch January 4, 2018 17:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants