-
-
Notifications
You must be signed in to change notification settings - Fork 293
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
Deprecate node 14 #2195
Deprecate node 14 #2195
Conversation
🦋 Changeset detectedLatest commit: 895ad5c The changes in this PR will be included in the next version bump. This PR includes changesets to release 32 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Maybe this should actually be a minor bump. |
I think changing the version in engines is technically a breaking change |
Some of these versions will be |
Ah! I thought there was another PR doing similar things, but I couldn't find it! |
703573f
to
3e9ab02
Compare
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.
These versions look good to me, barring rollup-plugin-workbox
. This was one of the last pieces of the puzzle @43081j.
If we could get one more rebase @koddsson, I think we're good to go. We'll give a day for anyone to speak up on rollup-plugin-workbox
and then be just about ready for a real release!
'@web/test-runner-webdriver': minor | ||
--- | ||
|
||
Set node 16 as the minimum version. |
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.
rollup-plugin-workbox
isn't included here. It doesn't have tests, so it might not actually be "effected" by this change, even though it is. Anyone have thoughts on this?
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'm probably missing something, but why should it be included in this list?
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.
The reason to include it would be that the GitHub Action no longer including 14 effects ALL packages, even though it isn't directly in their directory tree. Nothing supports 14 any more, so everything is breaking.
The reason not to include it would be that while the above is true, there don't seem to be any tests for this package that would have run in 14, so we never technically supported 14 to begin with?
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 bumping it then makes sense. If not, just to make sure, we break something in an unrelated version bump later on.
5f85257
to
8536ba0
Compare
Co-authored-by: Westbrook Johnson <westbrook.johnson@gmail.com>
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.
Looks good to me!!
Node 14 is reaching End Of Life soon.
References
https://nodejs.dev/en/about/releases/
Closes #2162