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

ci: remove node 12 from CI tests#4029

Merged
MicaiahReid merged 4 commits intodevelopfrom
remove-node-12
Dec 15, 2022
Merged

ci: remove node 12 from CI tests#4029
MicaiahReid merged 4 commits intodevelopfrom
remove-node-12

Conversation

@MicaiahReid
Copy link
Copy Markdown
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.

Comment thread UPGRADE-GUIDE.md
Comment thread .github/workflows/push.yml Outdated
Comment thread UPGRADE-GUIDE.md Outdated
Copy link
Copy Markdown
Contributor

@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
Copy Markdown
Contributor

@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.

Comment thread .github/workflows/push.yml
Comment thread UPGRADE-GUIDE.md

#### 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
Copy Markdown
Contributor

@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
Copy Markdown
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