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

Test against more relevant versions of node on Travis #251

Merged
merged 4 commits into from May 14, 2016

Conversation

JaKXz
Copy link
Member

@JaKXz JaKXz commented May 8, 2016

allow for node@unstable failures

  • slightly quicker npm install

- allow for `node@unstable` failures
- slightly quicker npm install
@JaKXz JaKXz changed the title Test against more relevant versions of node Test against more relevant versions of node on Travis May 8, 2016
Waiting on nvm-sh/nvm#1053 for nightly installs.
@JaKXz
Copy link
Member Author

JaKXz commented May 8, 2016

Which of these compilers would be preferred?

@bcoe
Copy link
Member

bcoe commented May 9, 2016

@JaKXz I'm cool with dropping iojs and upgrading to 6, but since nyc is aiming to be fairly backwards compatible could you leave node@0.10.

@JaKXz
Copy link
Member Author

JaKXz commented May 9, 2016

@bcoe done. I'll leave the Travis warning about GCC compilers to you / the other maintainers if there's some preferred one necessary - or I can just add whichever you guys want. I don't know the differences well enough to make a substantial decision.

- "stable"

before_install:
- "npm config set progress=false"
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this do anything? npm won't render a progress bar in Travis anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

@novemberborn I think it does speed up install a bit, it definitely does locally... I don't think there's harm in having it on CI since like you said Travis doesn't render one anyway, but if there's even a slight benefit then it's worth it imho.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think there are any benefits for Travis (and I doubt whether there's significant ones locally). Best not to have invocations like this in the code, it'll only lead to more questions in the future.

@novemberborn
Copy link
Contributor

Should leave 0.12 as well, it's supported to the end of the year (and even afterwards we should refrain from unintentionally breaking compatibility). I'd prefer testing 5 while it's still being maintained.

@bcoe
Copy link
Member

bcoe commented May 10, 2016

@JaKXz thanks for entertaining our 🚲 🏠, will land this today :)

@JaKXz
Copy link
Member Author

JaKXz commented May 10, 2016

@bcoe TBPH I don't mind at all because this is really a discussion for you guys to have as maintainers (about which versions you're supporting etc) - I just noticed that 1) the builds were really slow and 2) that the versions of node were [well] out of date so I thought I'd kickstart things :)

@JaKXz
Copy link
Member Author

JaKXz commented May 10, 2016

@novemberborn this and the continuing discussion at npm/npm#11283 are why I suggest the before_install command - I don't mind being vetoed here but I don't think it's harmful.

@bcoe
Copy link
Member

bcoe commented May 11, 2016

@JaKXz easy enough to test; let's kick off a build with and without the setting, and look at the Travis benchmarks; there's a second count beside each step in the build.

@bcoe bcoe merged commit 53e683b into istanbuljs:master May 14, 2016
@JaKXz JaKXz deleted the patch-1 branch May 14, 2016 20:57
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

3 participants