-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
Use Node LTS in Travis #1736
Use Node LTS in Travis #1736
Conversation
The better way to do this might be to reference "lts". See GitbookIO/gitbook-cli#60 |
I didn't know you could do this... Where did you find it ? The document on Travis doesn't mention it : https://docs.travis-ci.com/user/languages/javascript-with-nodejs/#Specifying-Node.js-versions |
Hmm, seems like they should add that to the docs. I don't remember where I learned about it |
I opened a PR to add it to the npm docs: travis-ci/docs-travis-ci-com#1306 |
@AaronO if you're able to merge this PR as well it should fix the Travis build at least. I'm afraid the Appveyor build is harder to fix |
@weitzman @mintbridge @mcveat @ludovicofischer @AaronO would one of you be able to merge this PR to fix the Travis build? |
@weitzman @mintbridge @mcveat @ludovicofischer @AaronO would you be able to merge this to fix Travis? |
Unfortunately I'm not a GitBook maintainer... 😞 |
@AaronO @jpreynat @SamyPesse @Soreine @zhouzi would you be able to merge this to fix Travis? |
@Mogztter my change was merged to the Travis docs if you want to update this PR to use the latest lts syntax |
Build against LTS: https://github.com/nodejs/LTS#lts-schedule
@benmccann Done 😉 |
@Mogztter you did something to mess up this PR. It has hundreds of commits on it now |
@benmccann I see only one commit... ? |
After my comment it says "AaronO changed the base branch to GitbookIO:4.x.x from GitbookIO:master 8 hours ago". I'm guessing that fixed it |
@benmccann That's my fault. I moved the old
|
@AaronO does that mean we need to have this PR merged to both |
@benmccann PRs that are focused on fixes should be merged into master first and foremost. Since this is a "meta" / build related issue it should be merged into both (with master still being the priority) |
@AaronO thanks for the clarification. it sounds like this should be merged then? |
Node.js 6.x is now the "active" LTS: https://github.com/nodejs/LTS#lts-schedule
Plus Travis seems to have trouble building the project with Node 4.1