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: stop testing against NodeJS v14, v16 #849

Merged
merged 11 commits into from
Jun 12, 2023
Merged

Conversation

wolfy1339
Copy link
Member

@wolfy1339 wolfy1339 commented May 13, 2023

BREAKING CHANGE: Drop support for NodeJS v14, v16

@ghost ghost added this to Inbox in JS May 13, 2023
@wolfy1339 wolfy1339 added Type: Maintenance Any dependency, housekeeping, and clean up Issue or PR Type: Breaking change Used to note any change that requires a major version bump and removed maintenance labels May 13, 2023
@ghost ghost moved this from Inbox to Maintenance in JS May 13, 2023
@wolfy1339 wolfy1339 marked this pull request as draft May 13, 2023 14:56
@wolfy1339 wolfy1339 force-pushed the remove-eol-node-versions branch 6 times, most recently from d4f5f23 to 08fda2f Compare May 13, 2023 15:52
@wolfy1339 wolfy1339 marked this pull request as ready for review May 13, 2023 16:05
Copy link
Contributor

@gr2m gr2m left a comment

Choose a reason for hiding this comment

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

I think I'd remove both node 14 and node 16. Node 16 is nearly out of maintenance: https://nodejs.dev/en/about/releases/. If anyone complaints, they can still use the older versions and we can still accept patches for older versions. That will save us another breaking change release within a few months

Also when doing breaking changes, I'd always look for deprecations and remove them as well.

/cc @octokit/js

@wolfy1339
Copy link
Member Author

I think I'd remove both node 14 and node 16. Node 16 is nearly out of maintenance: https://nodejs.dev/en/about/releases/.

I hadn't really thought of Node 16

If anyone complaints, they can still use the older versions and we can still accept patches for older versions. That will save us another breaking change release within a few months

Good point. I'll update my octoherd script and run it again with these changes

@G-Rath
Copy link
Member

G-Rath commented May 13, 2023

Inversely: why are we dropping support for v14? I know that it'll mean we might not have to transpile as much, but I wouldn't expect the wins to be that great so unless there's a particular feature or dependency we know we can be using with v14/v16, or there's another breaking change we want to land (which might be #841?), is there a good reason to drop support for these versions?

@wolfy1339
Copy link
Member Author

wolfy1339 commented May 14, 2023

We will still support those versions with bug fixes for the near future.

Dropping support means easier transition to ESM, and no need to keep polyfills for the fetch API.

It just doesn't make much sense to support versions that are EOL

There was a discussion around this briefly, https://github.com/orgs/octokit/discussions/21

@wolfy1339 wolfy1339 changed the title ci: stop testing against NodeJS v14 ci: stop testing against NodeJS v14, v16 May 14, 2023
@G-Rath
Copy link
Member

G-Rath commented May 14, 2023

We will still support those versions with bug fixes for the near future.

In that case we should be testing against them? You either are or are not supporting the versions right? 🙂

There was a discussion around this briefly, https://github.com/orgs/octokit/discussions/21

It doesn't look like anything was ever actually agreed from that discussion - I've posted a comment with my thoughts on it.

@wolfy1339
Copy link
Member Author

In that case we should be testing against them? You either are or are not supporting the versions right? 🙂

As gr2m said, we can support them with patches in maintenance releases. In the current release, we won't be supporting them

If anyone complaints, they can still use the older versions and we can still accept patches for older versions.

kfcampbell
kfcampbell previously approved these changes May 19, 2023
@nickfloyd
Copy link
Contributor

@kfcampbell kfcampbell merged commit ea6f1f9 into main Jun 12, 2023
7 checks passed
@kfcampbell kfcampbell deleted the remove-eol-node-versions branch June 12, 2023 21:17
@github-actions
Copy link
Contributor

🎉 This PR is included in version 12.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released Type: Breaking change Used to note any change that requires a major version bump Type: Maintenance Any dependency, housekeeping, and clean up Issue or PR
Projects
Archived in project
JS
  
Maintenance
Development

Successfully merging this pull request may close these issues.

None yet

5 participants