-
Notifications
You must be signed in to change notification settings - Fork 237
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
Fixing CI for Node 4 #326
Fixing CI for Node 4 #326
Conversation
I'm okay with switching to npm, what do you think @dylang? |
@lorenzleutgeb I have updated the PR as requested. I haven't removed the old node versions that are EOL right now (4 and 6) because that would require a major version bump of npm-check If this passes we can at least get it merged and then decide to drop node 4 and node 6 support later 👍 |
Not sure if removing a testing environment from build automation constitutes a breaking change. No interface to the outside world is being changed. But I get your point that it would be more diligent to test with Node 4 as well :) Now let's hope some maintainers are around... |
I am part of one of the Ember core teams and we have a policy that when you stop testing a particular version you're dropping support for that version. Essentially if anyone adds a small change (like arrow functions) you won't notice that it breaks the build in Node 4 because you're not testing it anymore 😄 It's why I wanted to fix CI in the first place so that it was still a clear intent of versions that you want to support 👍 |
Agreeing with @mansona here, it is very common that JS libraries bump up a major version when they drop support for an older node version. |
Thanks!
Yeah, this is definitely how it should be done 👍 |
I noticed that CI was broken on one of my other PRs #323 because apparently Yarn doesn't support Node 4
I'm not trying to open a can of worms with the whole "npm vs Yarn" debate, all I know is that CI is currently broken and this PR fixes the issue 👍
I also added testing in Node 8 and 10 for good measure 🎉