-
Notifications
You must be signed in to change notification settings - Fork 22
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
chore: limit to Node >=9.4.0 || >8.9.4 <9 #4
Conversation
There was a breaking change in http2 with Node 9.4.0. A similar change will be made in Node 8 too. Temporarily disable CircleCI tests with Node 8. And also update README with the info.
Codecov Report
@@ Coverage Diff @@
## master #4 +/- ##
=======================================
Coverage 76.31% 76.31%
=======================================
Files 1 1
Lines 38 38
Branches 9 9
=======================================
Hits 29 29
Misses 9 9 Continue to review full report at Codecov.
|
I'm releasing the new version together with this change. The draft release note is in the GitHub UI. |
You can land changes like these with a TBR. |
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.
LGTM
- image: node:8 | ||
user: node | ||
<<: *unit_tests | ||
# node8: |
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.
I think technically you don't need to disable this here, right?
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.
You're right! But I'll leave this as disabled just to make it more prominent :)
There was a breaking change in http2 with Node 9.4.0. A similar change
will be made in Node 8 too.
Temporarily disable CircleCI tests with Node 8. And also update README
with the info.