Skip to content
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

Override travis install method to use npm for older versions of node #994

Merged
merged 3 commits into from
Nov 21, 2016
Merged

Conversation

olingern
Copy link
Contributor

@olingern olingern commented Nov 21, 2016

I noticed that my PR was failing on node v4.xx.x on Travis CI, so I decided to look into it briefly.

Travis-CI introduced support for Yarn recently. Unfortunately, Yarn doesn't officially support node versions < v6.xx.x

Purposed fix
Override Travis install method to use npm install for node versions < v6.xx.x

@olingern olingern changed the title override travis install method to use npm for older versions of node Override travis install method to use npm for older versions of node Nov 21, 2016
@brendankenny
Copy link
Member

brendankenny commented Nov 21, 2016

from a failed travis run, it looks like the issue is one of our dependencies (devtools-timeline-model) specifying an engines entry (">=5.0") in its package.json and the fact that yarn enforces that by default.

It looks like there is a --ignore-engines flag for yarn which would help here, but we'd still have to rewrite the install section.

Another option is just doing

install:
  npm install

until we're ready to ditch older nodes (#449) and/or all-in on yarn.

@olingern
Copy link
Contributor Author

I think the npm install option is good as it essentially reverts to the previous Travis-CI install method. It's unfortunate to not be able to take advantage of Yarn's caching, but --ignore-engines could potentially introduce more headaches from other dependencies.

Copy link
Member

@brendankenny brendankenny left a comment

Choose a reason for hiding this comment

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

awesome, thanks for catching this so quickly!

@brendankenny brendankenny merged commit bef8644 into GoogleChrome:master Nov 21, 2016
andrewrota pushed a commit to andrewrota/lighthouse that referenced this pull request Jan 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants