-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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(eslint-plugin): add rule ban-tslint-comment
#2140
feat(eslint-plugin): add rule ban-tslint-comment
#2140
Conversation
Thanks for the PR, @drewwyatt! 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. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day. |
add a lint rule to flag tslint rule-flags (e.g. tslint:disable) to be used when migrating from tslint to eslint
…from comment texts
Codecov Report
@@ Coverage Diff @@
## master #2140 +/- ##
==========================================
+ Coverage 93.44% 93.45% +0.01%
==========================================
Files 171 172 +1
Lines 7821 7838 +17
Branches 2229 2232 +3
==========================================
+ Hits 7308 7325 +17
Misses 245 245
Partials 268 268
|
Is there anything I can/should do re: codecov failure? |
@bradzacher updated. thanks for the feedback! |
@bradzacher is there anything this still needs for you to be able to resolve the change request? |
Nothing you need to do - it's in the review queue. I usually get around to reviews on the weekend 😄 |
one last question before merge |
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.
thanks for your work!
ban-tslint-comment
is there any interest in making this a default (well, " |
I'd love to for sure, but many of our users are still transitioning from tslint via our |
@bradzacher @dimitropoulos Could it make sense to recommend as a |
I definitely think this rule has value, and eventually I'd love to add it to recommended. But right now there are a non-trivial number of users using tslint (~3.2m weekly downloads, compared to our ~5.2m and typescript's ~11.9m), so I think it's too early to be forcing this on users. Ideally I'd be looking for the download counts to be ~10% of the ecosystem before I turn it on. We'll have to do a 4.0 a lot faster than we did a 3.0 due to #2204, so it definitely won't be in 4.0 |
When migrating from tslint to eslint it's probably somewhat common to have leftover tslint rule flags. This rule aims to help clean those up.
Edit: Apologies for the force-push (I know the CONTRIBUTING guide says to try not to do that). My first commit message was incorrect, hopefully since this wasn't a code change it is okay. 🙏