-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
ci(eslint): Inline all the external eslint-config-sentry* ESLint configs into our .eslintrc file #80970
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
Conversation
…lintrc.js I did skip the `no-restricted-imports` rule, because its just a proxy to eslint-config-sentry/index.js
…quired eslint packages Notice that: - Cleaned up a duplicate `argsIgnorePattern` in the original - `eslint-plugin-jest` went from resolved version `28.8.3` to `28.9.0` -> https://github.com/jest-community/eslint-plugin-jest/releases/tag/v28.9.0 - `eslint-plugin-react` went from resolved version `7.37.1` to `7.37.2` - We're getting some new resolutions in the yarn.lock file
note that we still have `settings.react.version: 17.0.2`. It was wrong before but i'm leaving it. Enabling it will cause passwordStrength.tsx to error with `ReactDOM.render is deprecated`
Note that previously `no-else-return` was defined twice.
Once as:
```
'no-else-return': ['off'],
```
and again as:
```
'no-else-return': ['error', {allowElseIf: false}],
```
The latter remains
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #80970 +/- ##
========================================
Coverage 78.49% 78.49%
========================================
Files 7210 7216 +6
Lines 319520 319772 +252
Branches 43983 44030 +47
========================================
+ Hits 250796 250995 +199
- Misses 62332 62388 +56
+ Partials 6392 6389 -3 |
| // https://eslint.org/docs/rules/space-in-brackets.html | ||
| 'object-curly-spacing': ['error', 'never'], |
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.
would be nice to follow up by removing any of the rules that are now in @stylistic/eslint-plugin-js since they are overwritten or disabled by prettier anyway
they should all have the deprecation notice in their docs https://eslint.org/docs/latest/rules/object-curly-spacing
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.
there's so much to cleanup. I didn't even touch the react version: '17.0.2' bit yet :(
package.json
Outdated
| "benchmark": "^2.1.4", | ||
| "eslint": "8.57.1", | ||
| "eslint-config-sentry-app": "2.9.0", | ||
| "eslint-config-sentry": "^2.10.0", |
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.
can we remove this now?
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.
yup! the last step.
we do still need the plugin though, just not this config.
…the plugin package however
.eslintrc.js
Outdated
| }; | ||
|
|
||
| const extendsList = [ | ||
| 'sentry', |
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 its complaining about this
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.
d'oh, missed in 00fae01
Bundle ReportChanges will increase total bundle size by 37.81kB (0.12%) ⬆️. This is within the configured threshold ✅ Detailed changes
|
…igs into our .eslintrc file (#80970) Related to https://github.com/getsentry/frontend-tsc/issues/81 I'm proposing that we inline all this eslint config into the repo, instead of leveraging an external package. The idea is to make it easier and faster to iterate on. This PR is just an naive 7 mechanical operation: inline everything and don't refactor too much. Each commit in this PR should be easy to compare against https://github.com/getsentry/eslint-config-sentry with some small things to note along the way. The followup task will be https://github.com/getsentry/frontend-tsc/issues/82 which is a good time to squash objects and generally have a reason to refactor what's in here. Possible strategies might be to stratify the config values based on some/all of: - plugin name - runtime speed ie: `import slowRules from './eslint/slowRules.js';` - etc.
…igs into our .eslintrc file (#80970) Related to getsentry/frontend-tsc#81 I'm proposing that we inline all this eslint config into the repo, instead of leveraging an external package. The idea is to make it easier and faster to iterate on. This PR is just an naive 7 mechanical operation: inline everything and don't refactor too much. Each commit in this PR should be easy to compare against https://github.com/getsentry/eslint-config-sentry with some small things to note along the way. The followup task will be getsentry/frontend-tsc#82 which is a good time to squash objects and generally have a reason to refactor what's in here. Possible strategies might be to stratify the config values based on some/all of: - plugin name - runtime speed ie: `import slowRules from './eslint/slowRules.js';` - etc.
…igs into our .eslintrc file (#80970) Related to getsentry/frontend-tsc#81 I'm proposing that we inline all this eslint config into the repo, instead of leveraging an external package. The idea is to make it easier and faster to iterate on. This PR is just an naive 7 mechanical operation: inline everything and don't refactor too much. Each commit in this PR should be easy to compare against https://github.com/getsentry/eslint-config-sentry with some small things to note along the way. The followup task will be getsentry/frontend-tsc#82 which is a good time to squash objects and generally have a reason to refactor what's in here. Possible strategies might be to stratify the config values based on some/all of: - plugin name - runtime speed ie: `import slowRules from './eslint/slowRules.js';` - etc.
…igs into our .eslintrc file (#80970) Related to getsentry/frontend-tsc#81 I'm proposing that we inline all this eslint config into the repo, instead of leveraging an external package. The idea is to make it easier and faster to iterate on. This PR is just an naive 7 mechanical operation: inline everything and don't refactor too much. Each commit in this PR should be easy to compare against https://github.com/getsentry/eslint-config-sentry with some small things to note along the way. The followup task will be getsentry/frontend-tsc#82 which is a good time to squash objects and generally have a reason to refactor what's in here. Possible strategies might be to stratify the config values based on some/all of: - plugin name - runtime speed ie: `import slowRules from './eslint/slowRules.js';` - etc.
Related to https://github.com/getsentry/frontend-tsc/issues/81
I'm proposing that we inline all this eslint config into the repo, instead of leveraging an external package. The idea is to make it easier and faster to iterate on.
This PR is just an naive 7 mechanical operation: inline everything and don't refactor too much. Each commit in this PR should be easy to compare against https://github.com/getsentry/eslint-config-sentry with some small things to note along the way.
The followup task will be https://github.com/getsentry/frontend-tsc/issues/82 which is a good time to squash objects and generally have a reason to refactor what's in here. Possible strategies might be to stratify the config values based on some/all of:
import slowRules from './eslint/slowRules.js';