Skip to content

test: add ESLint 8.x tests #200

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 17, 2022
Merged

test: add ESLint 8.x tests #200

merged 1 commit into from
Jan 17, 2022

Conversation

MichaelDeBoey
Copy link
Member

@MichaelDeBoey MichaelDeBoey commented Sep 16, 2021 •
edited
Loading

ESLint v8.0.0 is released 🎉

Closes #190
Closes #252
Closes #253

@MichaelDeBoey MichaelDeBoey added enhancement New feature or request BREAKING CHANGE This change will require a major version bump labels Sep 16, 2021
@MichaelDeBoey MichaelDeBoey mentioned this pull request Sep 16, 2021
Merged
63 tasks
@MichaelDeBoey MichaelDeBoey self-assigned this Sep 29, 2021
@kimromi kimromi mentioned this pull request Oct 12, 2021
Closed
@nzdjb nzdjb mentioned this pull request Oct 23, 2021
Closed
@weyert
Copy link

weyert commented Nov 25, 2021

Anyone know what the status of this is?

@MichaelDeBoey MichaelDeBoey force-pushed the eslint-8 branch 2 times, most recently from 7365907 to 0aa330d Compare November 25, 2021 15:40
@codecov
Copy link

codecov bot commented Nov 25, 2021 •
edited
Loading

Codecov Report

Merging #200 (9cf0e05) into main (1e665e6) will not change coverage.
The diff coverage is n/a.

❗ Current head 9cf0e05 differs from pull request most recent head 7839b8a. Consider uploading reports for the commit 7839b8a to get more accurate results
Impacted file tree graph

@@            Coverage Diff            @@
##              main      #200   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           15        15           
  Lines          563       560    -3     
  Branches       158       156    -2     
=========================================
- Hits           563       560    -3
Impacted Files Coverage Δ
src/assignment-ast.js 100.00% <0.00%> (ø)
src/rules/prefer-in-document.js 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e665e6...7839b8a. Read the comment docs.

@MichaelDeBoey MichaelDeBoey marked this pull request as ready for review November 25, 2021 15:48
@MichaelDeBoey MichaelDeBoey force-pushed the eslint-8 branch 4 times, most recently from 101be4c to fb64e98 Compare November 25, 2021 16:15
@G-Rath
Copy link
Collaborator

G-Rath commented Nov 25, 2021 •
edited
Loading

@weyert ESLint v8 is already supported, we're just not testing against it.

@MichaelDeBoey we shouldn't need to do a breaking release for this (though we might need to for supporting @testing-library/dom v8) - are you happy to update this PR to be only changes that are needed for testing against ESLint v8 in CI?

(Feel free to do PRs for the other changes, like updating TypeScript, as they're still valuable)

This was referenced Dec 13, 2021
Merged
Merged
Merged
Merged
@MichaelDeBoey
Copy link
Member Author

@G-Rath I've merged #235, #236, #237 & #238 and added one last commit to support ESLint v8.x

@MichaelDeBoey MichaelDeBoey marked this pull request as draft December 13, 2021 18:07
@MichaelDeBoey MichaelDeBoey force-pushed the eslint-8 branch 2 times, most recently from c9bbcad to f0abb56 Compare December 15, 2021 20:47
@G-Rath G-Rath mentioned this pull request Dec 19, 2021
Closed
@G-Rath
Copy link
Collaborator

G-Rath commented Dec 19, 2021

@MichaelDeBoey I think for now we should probably stick with just adding v8 to the matrix of eslint versions we test against - I don't think there's a need to change our version constraints for now (since you're technically tightening them)

@MichaelDeBoey MichaelDeBoey force-pushed the eslint-8 branch 2 times, most recently from ee3defa to b1e95a2 Compare December 19, 2021 03:06
@MichaelDeBoey MichaelDeBoey removed the BREAKING CHANGE This change will require a major version bump label Dec 19, 2021
@MichaelDeBoey MichaelDeBoey force-pushed the eslint-8 branch 2 times, most recently from 8f10391 to 662e5c7 Compare December 26, 2021 23:57
@MichaelDeBoey MichaelDeBoey marked this pull request as ready for review December 26, 2021 23:57
@MichaelDeBoey MichaelDeBoey force-pushed the eslint-8 branch 2 times, most recently from c2196c7 to 1aa9a85 Compare January 11, 2022 17:27
@MichaelDeBoey
Copy link
Member Author

@G-Rath Now that [email protected] is released, this one's good to be merged 🥳🎉

G-Rath
G-Rath requested changes Jan 14, 2022
View reviewed changes
Copy link
Collaborator

@G-Rath G-Rath left a comment

Choose a reason for hiding this comment

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

I don't think this should be feat since we're actually already compatible with ESLint v8, and that could give people looking at the changelogs the wrong impression.

Instead either ci or chore would be more appropriate.

package.json
},
"peerDependencies": {
"eslint": ">=6.8"
"eslint": "^6.8.0 || ^7.0.0 || ^8.0.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should leave the range as-is, since we might already be compatible with new eslint versions (e.g. we're already compatible with 8 as a plugin).

We can always constraint it in a bugfix release if we find we're not compatible in the future.

Suggested change
"eslint": "^6.8.0 || ^7.0.0 || ^8.0.0"
"eslint": ">=6.8"

Copy link
Member Author

Choose a reason for hiding this comment

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

This makes it more explicit that we're 100% supporting a certain version.

If ESLint v9 gets released, it will only be a small change here together with adding the tests

Copy link
Collaborator

Choose a reason for hiding this comment

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

Meanwhile developers would be stuck using ESLint v8 or lower until we were able to make that change, and we've already seen how hard it can be to add tests for a new ESLint version...

@MichaelDeBoey MichaelDeBoey changed the title feat: support ESLint 8.x test: add ESLint 8.x tests Jan 17, 2022
@MichaelDeBoey MichaelDeBoey requested a review from G-Rath January 17, 2022 17:21
@MichaelDeBoey MichaelDeBoey merged commit b84fadd into main Jan 17, 2022
@MichaelDeBoey MichaelDeBoey deleted the eslint-8 branch January 17, 2022 18:49
@github-actions
Copy link

🎉 This PR is included in version 4.0.2 🎉

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
Reviewers

@G-Rath G-Rath G-Rath approved these changes

@nickserv nickserv Awaiting requested review from nickserv

@benmonro benmonro Awaiting requested review from benmonro

Assignees

@MichaelDeBoey MichaelDeBoey

Labels
enhancement New feature or request released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support ESLint 8.x
3 participants
@MichaelDeBoey @weyert @G-Rath