-
Notifications
You must be signed in to change notification settings - Fork 2
Add ESLint 9 support and migrate to flat config #216
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
base: trunk
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 15 out of 30 changed files in this pull request and generated 2 comments.
Files not reviewed (15)
- .eslintignore: Language not supported
- .nvmrc: Language not supported
- tests/snapshots/check-fixtures.js.snap: Language not supported
- .eslintrc.js: Evaluated as low risk
- configs/weak-testing.js: Evaluated as low risk
- tests/check-fixtures.js: Evaluated as low risk
- tests/rules/nestjs-route-prefix.ts: Evaluated as low risk
- tests/rules/no-async-foreach.ts: Evaluated as low risk
- configs/cli.js: Evaluated as low risk
- configs/formatting.js: Evaluated as low risk
- configs/index.js: Evaluated as low risk
- configs/weak-typescript.js: Evaluated as low risk
- configs/jsdoc.js: Evaluated as low risk
- configs/weak-javascript.js: Evaluated as low risk
- configs/react.js: Evaluated as low risk
Comments suppressed due to low confidence (3)
configs/javascript.js:23
- The 'ecmaVersion' property is set twice in the 'languageOptions' object. The second instance on line 32 should be removed to avoid redundancy and potential conflicts.
ecmaVersion: 2022,
configs/testing.js:28
- The rule 'jest/unbound-method' is not a valid rule in the Jest plugin. The correct rule name is '@typescript-eslint/unbound-method'.
'jest/unbound-method': 'error',
configs/recommended.js:22
- The
debugLogfunction call has been removed. This function was used to log the configurations being used, which could be useful for debugging purposes. Consider re-adding this logging functionality.
module.exports = configs;
|
@copilot Please review again, particularly the feedback to your comments. |
|
@GaryJones I've opened a new pull request, #224, to work on those changes. Once the pull request is ready, I'll request review from you. |
9cf21a6 to
9ef85a0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 28 out of 30 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
rules/no-unguarded-get-range-at.js:11
- The
meta.messagesobject should be added with a message definition, and thecontext.reportcall on line 19 should usemessageIdinstead ofmessage. This ensures consistency with ESLint 9's rule API and matches the pattern used innestjs-route-prefix.js.
Example fix:
meta: {
type: 'problem',
docs: {
description: 'Avoid unguarded getRangeAt',
},
messages: {
unguarded: 'Avoid unguarded getRangeAt',
},
schema: [],
},Then update the report call to use messageId: 'unguarded'.
meta: {
type: 'problem',
schema: [],
},
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
6b31b47 to
8253530
Compare
Co-authored-by: Copilot <[email protected]>
88a9ccc to
a8fa6e2
Compare
a8fa6e2 to
8690a49
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 28 out of 30 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 28 out of 30 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@GaryJones I have created a PR in the VIP CLI repo to test this PR: Automattic/vip-cli#2650 It even seems to work 😆 |
This PR:
Unfortunately, these changes are breaking; consumers' configuration files will have to be migrated manually.
The plan is to release
1.0.0-rc@nextand test it on some of our code bases; once we ensure everything is good, publish 1.0.