-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
docs(eslint-plugin): [class-methods-use-this] refresh options docs #10728
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
docs(eslint-plugin): [class-methods-use-this] refresh options docs #10728
Conversation
Thanks for the PR, @JoshuaKGoldberg! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. |
✅ Deploy Preview for typescript-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
View your CI Pipeline Execution ↗ for commit fc45117.
☁️ Nx Cloud last updated this comment at |
|
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.
This is such a big improvement, thank you 🎉
@@ -33,8 +33,6 @@ const defaultOptions: Options = { | |||
|
|||
{/* insert option description */} | |||
|
|||
Makes the rule ignore any class member explicitly marked with `override`. |
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.
Intersting!
I just noticed the resulting documentation page had more-or-less the same option description but twice, as the /* insert option description */
comments add the original option description.
I've checked if there are other rules that have this issue which led me to #10748. Some rules have options with exactly the same description which seems rather confusing (for example: https://typescript-eslint.io/rules/no-confusing-void-expression/#ignorearrowshorthand).
I wonder if/how this can be prevented.
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.
Yeah we could probably add a lint rule or test to check that there isn't a strict duplicate. Or loose duplication if we remove spaces, lowercase, or even check something like the Levenshtein distance and/or are-docs-informative
-style distance. But there are so many that are very differently phrased -and we aren't adding new violations- I don't think it'd catch much long-term. +1 to #10748 from me as a one-time thing.
cd15629
into
typescript-eslint:main
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10728 +/- ##
=======================================
Coverage 87.26% 87.27%
=======================================
Files 450 450
Lines 15712 15719 +7
Branches 4600 4601 +1
=======================================
+ Hits 13711 13718 +7
Misses 1645 1645
Partials 356 356
Flags with carried forward coverage won't be shown. Click here to find out more.
|
PR Checklist
Overview
Aligns the rule's options' descriptions with how most other
ignore*
ones are described.💖