-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat: update eslint to v8 #18264
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: develop
Are you sure you want to change the base?
feat: update eslint to v8 #18264
Conversation
b779b41 to
c8b1fd3
Compare
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
c8b1fd3 to
b2f7836
Compare
5b9d895 to
a6afd58
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.
Theoretically this is considered a breaking change. Since this is an internal @sentry-internal/* package, we can release this in a non-breaking fashion
Before we merge this, let's check in with Electron, Lynx and React-Native SDK maintainers. They all use a 10.x version of @sentry-internal/eslint-config-sdk so we should make sure this change doesn't cause problems for them:
https://github.com/search?type=code&q=%40sentry-internal%2Feslint-config-sdk+owner%3Agetsentry+
I agree btw that we shouldn't have to stick to semver for this internal package. Let's just make sure we break no-one unexpectedly 😅
b2ce5c4 to
de82cc5
Compare
|
@Lms24 I just asked internally and it seems to be ok for the others |
de82cc5 to
bbc1816
Compare
bbc1816 to
e6cd95c
Compare
This upgrades our internal eslint config to use v8. Theoretically this is considered a breaking change. Since this is an internal
@sentry-internal/*package, we can release this in a non-breaking fashionWith v8 couple of changes came. Some of them are affecting us, which required file changes:
Merge checks