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

Make some updates in Travis config #2104

Open
wants to merge 1 commit into
base: master
from

Conversation

@chenrui333
Copy link

commented Oct 2, 2019

No description provided.

@@ -12,7 +11,6 @@ addons:
cache:
ccache: true
directories:
- $HOME/.npm

This comment has been minimized.

Copy link
@chenrui333

chenrui333 Oct 2, 2019

Author

This is now cached by default.

Please note that as of July 2019, npm is cached by default on Travis CI

This comment has been minimized.

Copy link
@ljharb

ljharb Oct 2, 2019

Member

i still prefer the explicit entry; but can you link to where this is quoted from?

This comment has been minimized.

Copy link
@chenrui333

This comment has been minimized.

Copy link
@chenrui333

chenrui333 Oct 3, 2019

Author

I do see the value of being explicit, but the cache policy change is the new norm though :)

This comment has been minimized.

Copy link
@ljharb

ljharb Oct 3, 2019

Member

That says

This caches $HOME/.npm or node_modules, depending on the repository’s structure

which makes me more strongly prefer the explicit form.

@@ -1,6 +1,5 @@
language: generic
dist: xenial
sudo: required

This comment has been minimized.

Copy link
@chenrui333

chenrui333 Oct 2, 2019

Author

This is not quite needed.

This comment has been minimized.

Copy link
@ljharb

ljharb Oct 2, 2019

Member

why not? switching from xenial to bionic may have lots of implications about what can be compiled.

This comment has been minimized.

Copy link
@chenrui333

chenrui333 Oct 3, 2019

Author

That might be true, but the sudo: required is not needed though.

This comment has been minimized.

This comment has been minimized.

Copy link
@ljharb

ljharb Oct 3, 2019

Member

i'm fine removing the sudo key, only.

@ljharb ljharb added the testing label Oct 2, 2019
@chenrui333

This comment has been minimized.

Copy link
Author

commented Oct 3, 2019

Looks like some build failures, some did not.

@chenrui333

This comment has been minimized.

Copy link
Author

commented Oct 3, 2019

I will see how to do the cross-linux dist test tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.