Skip to content
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

ci: drop node.js 14.x and 16.x testing #536

Merged
merged 3 commits into from
Oct 23, 2024

Conversation

MikeMcC399
Copy link
Contributor

Issue

  • Node.js 14.x and 16.x reached Node.js End-of-life more than one year ago and so are unsupported by Node.js.
  • To lint TypeScript in a flat configuration, a minimum @typescript-eslint/eslint-plugin@7.0.0 is required, which in turn requires Node.js ^18.18.0. This rules out Node.js 14.x and 16.x.

Change

@MikeMcC399 MikeMcC399 marked this pull request as ready for review October 22, 2024 11:36
@ddzz
Copy link
Collaborator

ddzz commented Oct 22, 2024

We should remove 14 and 16 from the engines in the package.json as well.

@MikeMcC399

This comment was marked as outdated.

@ddzz ddzz requested a review from bmish October 22, 2024 12:20
@ddzz
Copy link
Collaborator

ddzz commented Oct 22, 2024

@bmish Can you review this PR and its related issues when you have a moment?

@MikeMcC399
Copy link
Contributor Author

MikeMcC399 commented Oct 22, 2024

I was originally planning to delay modifying engines until the repo is compatible with ESLint v9, then change the prerequisites of eslint and engines together. I guess the order does not matter too much so long as no new version is released. Modifying engines would be a breaking change anyway.

@ddzz
Copy link
Collaborator

ddzz commented Oct 23, 2024

@bmish is MIA so I'll approve and merge this. We should add Node 20.x to the matrix in another PR. Thanks for all your work in this repo!

@ddzz ddzz merged commit f108ee1 into bmish:main Oct 23, 2024
2 checks passed
@MikeMcC399
Copy link
Contributor Author

@ddzz

@bmish is MIA so I'll approve and merge this. We should add Node 20.x to the matrix in another PR. Thanks for all your work in this repo!

Thank you! This makes it cleaner for the follow-on PRs. I'm happy to work on this knowing you are there to review and merge. 🙂 I don't think we need @bmish at this time. He may still be traveling.

@MikeMcC399 MikeMcC399 deleted the remove/node-14-and-16-in-ci branch October 23, 2024 13:33
@MikeMcC399
Copy link
Contributor Author

MikeMcC399 commented Oct 23, 2024

@ddzz

We should add Node 20.x to the matrix in another PR.

Agreed, however the workflow is not yet compatible with Node.js >= 20 (otherwise I would have already added it). I am however planning to look at this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drop Node.js 14.x and 16.x testing in CI
2 participants