-
Notifications
You must be signed in to change notification settings - Fork 144
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
feat: cleanup main types #964
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #964 +/- ##
==========================================
+ Coverage 96.23% 96.32% +0.09%
==========================================
Files 44 46 +2
Lines 2419 2479 +60
Branches 1000 1017 +17
==========================================
+ Hits 2328 2388 +60
Misses 91 91 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
I don't really see this as strictly better - the types as they are pretty straightforward and maintainable as they are imo, and I in particular like having the configs written out individually to make it clear what is available without having to mentally unroll some type logic. Unlike your PRs to the other plugins, this one does has a slight advantage due to the number of configs, but given I don't expect this to need changing pretty much ever (aside from when the At the very least, this is not a "feat" or a "fix" |
@G-Rath We're also using TSESLint's types here, which is a dependency we already have If we use ESLint's dependency, we would need to add |
@MichaelDeBoey you'll already need |
@G-Rath with these changes, you don't need |
@MichaelDeBoey do you have an actual example of a project where you are getting an error with these types because Because I'm willing to bet most people will have So the only situation I can think of where this'd come up is if you're running just this plugin with ESLint v8 which seems kind of weird - why would you go to all the trouble of setting up ESLint just to apply linting rules for your testing code? Meanwhile, everyone is busy upgrading to ESLint v9 which as mentioned before is now typed - so I feel the audience for this is very niche (and that ultimately installing |
Follow-up of #963
CC/ @G-Rath