-
-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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
Add Node 10 to Travis config and remove Node 6 #4383
Conversation
Appveyor config also needs to be updated but I'm not sure if they support Node 10 yet. Actually I'm not sure if Travis does either so let's see what happens. 😀🍿 |
Both support it. Can you update/rebase the PR and trigger a new build? |
Do we plan to remove support for Node 6? |
I will merge some of our recent test improvements into this branch which will re-trigger the tests. |
Cool. I'm cool with dropping it |
For context, I believe we dropped Node 4 when it entered maintenance (LTS expired), but before EOL. Normally I'd want to wait for EOL, but the difference in features and speed between 6/8 is so significant I don't think it's worth keeping around until EOL. :) |
5fbd6d5
to
0b1ae21
Compare
.travis.yml
Outdated
@@ -4,6 +4,7 @@ language: node_js | |||
node_js: | |||
- 8 | |||
- 9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove Node 9 and merge this with next
please?
51df705
to
28032f2
Compare
Ugh. Tests passed on Node 10 for simple and monorepo but failed for install and kitchensink. They just timed out. For the simple test suite it took twice as long to run on Node 10 as it did on Node 8. Still want me to merge this? |
I want to get a PR passing before we merge ... what in the world is going on. 😅 |
I don't know but it's a real mess. Sometimes Travis fails, sometimes Appveyor fails, sometimes they both fail and every once in a while they both miraculously pass. Often they will go from passing to failing and the only difference is a change in the README. 🤷♂️ |
Sounds like some race conditions. |
Can you add |
This seems like a bit of an improvement. Kitchensink is very flaky on both Node 8 and 10. It passed on Travis but only because I re-ran it 4 times. It seems to fail in different places with different errors. Installs is consistently failing on Node 10 at the same place on Travis and Appveyor, although it fails in different ways on both. This could be due to the different version of yarn. This is what the logs look like on Travis and it just hangs here for 30+ minutes until the test times out:
On Appveyor it actually fails on the same part and this is what the logs look like there:
|
Can we install the latest Yarn? |
I added a I'm trying to figure out if I can skip installing Yarn with Node 0.10. Alternatively we could update our |
ee5e58b
to
ac7031f
Compare
Looks like all the tests are now passing on Travis. I had to restart two of them because they failed to install the latest version of Yarn. It looked like a temporary issue with the Yarn site unrelated to our tests. Appveyor tests are queued... 🍿 |
Appveyor!! |
a91262f
to
c5bb386
Compare
c5bb386
to
9828d6c
Compare
It works! 🎉 Updating Yarn seems to make a difference with Appveyor. On Travis the process of updating Yarn itself seems to be a bit flaky. The download failed a couple of times and there was a checksum error another couple of times. I'm not sure if this is an issue with the Yarn download site or with Travis. Either way, this seems like a big improvement. |
Should the readme be updated to reflect the drop of Node 6? Right now it states:
|
The README is based on CRA 1.0. We will be updating all the documentation before 2.0 is released. |
Update Travis config to add support for Node 10 and remove Node 6.