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

feat!: drop node v8 and v10 #203

Merged
merged 3 commits into from
Jun 25, 2022
Merged

Conversation

amareshsm
Copy link
Member

Description:

Drop node v8.x and v10.x

@nzakas
Copy link
Member

nzakas commented Jun 2, 2022

@btmills thoughts?

@btmills btmills added the tsc agenda This issue will be discussed by ESLint's TSC at the next meeting label Jun 2, 2022
@btmills
Copy link
Member

btmills commented Jun 2, 2022

TSC summary: eslint-plugin-markdown currently tests against ESLint 6+ and Node 8+. This pull request would drop support for end-of-life versions of Node.js. Normally, we drop support for EOL engines as part of regular major releases. However, we have no other breaking changes queued up for this package.

TSC question: Should we do a semver-major release just to drop support for EOL Node.js versions?

@btmills btmills removed the tsc agenda This issue will be discussed by ESLint's TSC at the next meeting label Jun 5, 2022
Copy link
Member

@btmills btmills left a comment

Choose a reason for hiding this comment

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

Thank you, @amareshsm! The team had no objections to doing a v3 with dropping EOL versions as the only breaking change.

It looks like there are some compat issues with Node 18. If necessary, feel free to drop 8/10/<12.22.0 here and split adding 18 support into another PR.

@amareshsm
Copy link
Member Author

Drafted a separate PR for node v18 support.

Copy link
Member

@btmills btmills left a comment

Choose a reason for hiding this comment

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

One inline change, then LGTM!

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@btmills btmills mentioned this pull request Jun 9, 2022
Co-authored-by: Brandon Mills <[email protected]>
@amareshsm
Copy link
Member Author

Bump

@btmills
Copy link
Member

btmills commented Jun 20, 2022

This and your other PR appear to be failing due to behavior changes in new npm versions that were released since the previous merges to main. Nothing about your change caused these failures. I've submitted #206 to fix the changes on main. Once that's in, I bet rebasing both of your branches will let them pass 👍

@btmills
Copy link
Member

btmills commented Jun 23, 2022

Closing and reopening to see if that'll fix CI without having to rebase now that #206 is merged.

@btmills btmills closed this Jun 23, 2022
@btmills btmills reopened this Jun 23, 2022
Copy link
Member

@btmills btmills left a comment

Choose a reason for hiding this comment

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

That's more like it! Thanks for your patience, @amareshsm. LGTM, but since this is a breaking change, I'll leave it open another couple days to give others a chance to review.

Copy link
Contributor

@snitin315 snitin315 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM

@nzakas nzakas merged commit 071fa66 into eslint:main Jun 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants