Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants