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

refactored travis configuration #494

Closed
wants to merge 1 commit into from

Conversation

@udnisap
Copy link

udnisap commented Nov 27, 2015

sudo is removed as it is the default configuration.
added v4 and v5 for building environments
caching is removed as it might have problems when we change the dependency tree.

@rom1504

This comment has been minimized.

Copy link
Member

rom1504 commented Nov 27, 2015

"caching is removed as it might have problems when we change the dependency tree.", I doubt this is a problem https://blog.travis-ci.com/2013-12-05-speed-up-your-builds-cache-your-dependencies/

@udnisap

This comment has been minimized.

Copy link
Author

udnisap commented Nov 27, 2015

Well the problem is say we upgrade a version of a dependency and since we are caching we will have both the libraries and may have problems when importing them as well when there is a peer dependency breaks a version of another library with a previous build.

@rom1504

This comment has been minimized.

Copy link
Member

rom1504 commented Nov 27, 2015

upgrade a version of a dependency and since we are caching we will have both the libraries

I'm not sure how travis caching work, but since they specifically put it as an example in that article up there, caching is probably smarter than that. I bet if you change the dependencies then the node_modules dir is deleted.

@udnisap

This comment has been minimized.

Copy link
Author

udnisap commented Nov 27, 2015

I have to admit I'm a bit stumped now. Travis isn't doing anything special when it comes to caching, we download, unzip, then zip, and upload.

travis-ci/travis-ci#3758 (comment)

@udnisap

This comment has been minimized.

Copy link
Author

udnisap commented Nov 27, 2015

and how about adding iojs too in the list ?

@josephfrazier

This comment has been minimized.

Copy link
Member

josephfrazier commented Nov 27, 2015

sudo is removed as it is the default configuration.

Not quite, see https://docs.travis-ci.com/user/workers/container-based-infrastructure/#Routing-your-build-to-container-based-infrastructure

caching is removed as it might have problems when we change the dependency tree.

I think there are only problems if npm install were to not overwrite old dependencies properly. See here for details on how caching works https://docs.travis-ci.com/user/caching/#How-does-the-caching-work%3F In any case, I think we should wait until there are actually problems with caching before removing it.

added v4 and v5 for building environments

Shouldn't v4 always resolve to the same version as 4.2? v5 and iojs sound nice though.

@udnisap

This comment has been minimized.

Copy link
Author

udnisap commented Nov 27, 2015

sudo is removed as it is the default configuration.
Not quite, see https://docs.travis-ci.com/user/workers/container-based-infrastructure/#Routing-your-build-to-container-based-infrastructure

I was referring to
https://docs.travis-ci.com/user/migrating-from-legacy/#tl%3Bdr

Shouldn't v4 always resolve to the same version as 4.2? v5 and iojs sound nice though.

It is but this will use the major version with the latest update

@josephfrazier

This comment has been minimized.

Copy link
Member

josephfrazier commented Nov 27, 2015

I was referring to
https://docs.travis-ci.com/user/migrating-from-legacy/#tl%3Bdr

That link points out that sudo: false is needed in order to use the container-based infrastructure (for pre-2015 repos). According to the Travis builds for this PR, removing sudo: false reverts to the legacy infrastructure: https://travis-ci.org/feross/webtorrent/jobs/93513185

It is but this will use the major version with the latest update

Oh, I see now. I had assumed in #483 that since 4.2 was an LTS release, there would be no further 4.x.0 releases, but that's not necessarily the case, according to item 7 of https://github.com/nodejs/LTS/blob/d576207e6aff6f10006a47c7f7cfd02906352b10/README.md#lts-plan

@feross

This comment has been minimized.

Copy link
Member

feross commented Dec 2, 2015

Hey guys,

Thanks for all the discussion about how to make the travis configuration better. Here's what I suggest:

  1. Enabling the tests for node v5 sounds good. I'd like to support all node LTS versions (currently 0.12 and 4) as well as the latest node version (5). (There's a cool trick to get the latest version of node by using "node" as the version.) I have a PR out for that here: #501
  2. We should remove the npm cache. These builds don't take very long and it's causing test failures. I removed the cache in #500 after it caused a failure. An old version of create-torrent continued to be used even though a new one exists on npm. Removing the cache option fixed it.
  3. We should leave "sudo: false" alone since it's required for projects created before the travis container infrastructure existed.

Since 1 and 2 are taken care of by other PRs I've sent, I'm going to close this issue now. Thanks again for looking into this, @udnisap, @rom1504, and @josephfrazier.

@feross feross closed this Dec 2, 2015
josephfrazier referenced this pull request Dec 2, 2015
It’s causing the tests to fail because there’s an old version of
create-torrent being used.
@lock lock bot locked as resolved and limited conversation to collaborators May 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.