-
Notifications
You must be signed in to change notification settings - Fork 545
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
fix(eslint-config): replace gts with prettier and eslint #1439
fix(eslint-config): replace gts with prettier and eslint #1439
Conversation
eslint.config.js
Outdated
"prefer-rest-params": "off", | ||
"@typescript-eslint/naming-convention": [ | ||
"error", | ||
quotes: [2, 'single', { avoidEscape: true }], |
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.
Some rules here differ from core repo. Is this intended to keep the changeset small?
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.
True, but I wasn't sure if there were rules (eg: the import rule) that were added for a specific reason in this repo.
I would prefer to have no differences between the two repos.
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.
I've updated the eslint.config file to match the core repo's configuration as close as possible.
The following is the difference between this and core to prevent eslint errors in existing code:
eqeqeq
set tooff
instead of["error", "smart"]
- removed
"no-console": "error"
- removed
"@typescript-eslint/no-floating-promises": "error"
- added
"@typescript-eslint/no-var-requires": "off"
- removed
"@typescript-eslint/no-unused-vars": ["error", {"argsIgnorePattern": "^_", "args": "after-used"}]
Let me know if it makes sense to re-add/remove these rules which will require some code changes in multiple packages in the repo.
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.
I think that's fine as a first step but we should aim to get in sync soon via some followup PRs. Best one by one to keep changesets small and easy/fast reviewed.
In special disabling eqeqeq
and no-console
allows code patterns we usually don't want.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1439 +/- ##
==========================================
- Coverage 96.13% 93.51% -2.63%
==========================================
Files 14 111 +97
Lines 906 5010 +4104
Branches 197 923 +726
==========================================
+ Hits 871 4685 +3814
- Misses 35 325 +290 |
Fixes #1433
Short description of the changes
eslint:recommended, plugin:@typescript-eslint/recommended, plugin:prettier/recommended
to match the core repo's configuration.