Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

ci: remove node 12 from CI tests #4029

Merged
merged 4 commits into from
Dec 15, 2022
Merged

ci: remove node 12 from CI tests #4029

merged 4 commits into from
Dec 15, 2022

Conversation

MicaiahReid
Copy link
Contributor

@MicaiahReid MicaiahReid commented Dec 14, 2022

You know when you spend a really long time on a big PR and finally think you've got every little detail settled, then the whole team reviews the PR and finds a few other little things that you fix, then they all approve the PR, so you all think you've got every little detail settled, then you finally merge the PR and you immediately realize that you forgot to remove a now unsupported version of node from your CI tests that only run once the PR has been merged into develop, so now you have to make another PR to remove the now unsupported node version from your CI tests? Yeah I hate when that happens.

UPGRADE-GUIDE.md Outdated Show resolved Hide resolved
UPGRADE-GUIDE.md Outdated Show resolved Hide resolved
Copy link
Member

@davidmurdoch davidmurdoch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see suggestions

Co-authored-by: David Murdoch <187813+davidmurdoch@users.noreply.github.com>
Copy link
Member

@cds-amal cds-amal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, @MicaiahReid, just one small nit, which won't hold back my approval.

.github/workflows/push.yml Show resolved Hide resolved

#### v7.7.0+, Dropped support for Node v12

We no longer support Node v12. You'll need to update to Node v14.0.0 or later. NOTE: Support for Node.js v14.x.x will be dropped shortly after the Node.js Foundation stops supporting it in April 2023.
Copy link
Member

@cds-amal cds-amal Dec 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
We no longer support Node v12. You'll need to update to Node v14.0.0 or later. NOTE: Support for Node.js v14.x.x will be dropped shortly after the Node.js Foundation stops supporting it in April 2023.
We no longer support Node v12. You'll need to update to Node v14 or later. NOTE: Support for Node.js v14 will be dropped shortly after the Node.js Foundation stops supporting it in April 2023.

I think the extra zeros and .x.x are confusing in this context.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree; this is saying you currently need at least the lowest v14, which is 14.0.0. And that we will drop support for all node v14 versions, 14.x.x

@MicaiahReid MicaiahReid merged commit 981e183 into develop Dec 15, 2022
@MicaiahReid MicaiahReid deleted the remove-node-12 branch December 15, 2022 16:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants