-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
feat(eslint-plugin): [ban-ts-comment] add descriptionFormat option #5026
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): [ban-ts-comment] add descriptionFormat option #5026
Conversation
✅ Deploy Preview for typescript-eslint ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Thanks for the PR, @Josh-Cena! 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. |
…r ts-expect-error
3ec575a
to
273e750
Compare
Codecov Report
@@ Coverage Diff @@
## main #5026 +/- ##
==========================================
+ Coverage 91.32% 93.79% +2.47%
==========================================
Files 132 286 +154
Lines 1487 9806 +8319
Branches 224 2933 +2709
==========================================
+ Hits 1358 9198 +7840
- Misses 65 328 +263
- Partials 64 280 +216
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
The code looks pretty reasonable at first glance. I think the API might need to be able to consider the original error code(s), as many users will ask for that?
Requesting changes more so that we can discuss it. Personally I'm on the fence.
Co-authored-by: Josh Goldberg <me@joshuakgoldberg.com>
Do note that this PR is purely stylistic and there's no semantic difference to tsc itself. |
type DirectiveConfig = | ||
| boolean | ||
| 'allow-with-description' | ||
| { descriptionFormat: string }; |
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.
Hmm, it would be nice to have minimumDescriptionLength
as an option in each object, so individual directives can override it... but that feels like a pretty niche use case. Maybe a followup to wait for someone to file an issue on.
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.
Fantastic, thanks @Josh-Cena! 💯
Great work! |
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | minor | [`5.26.0` -> `5.27.0`](https://renovatebot.com/diffs/npm/@typescript-eslint%2feslint-plugin/5.26.0/5.27.0) | | [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | minor | [`5.26.0` -> `5.27.0`](https://renovatebot.com/diffs/npm/@typescript-eslint%2fparser/5.26.0/5.27.0) | --- ### Release Notes <details> <summary>typescript-eslint/typescript-eslint (@​typescript-eslint/eslint-plugin)</summary> ### [`v5.27.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#​5270-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5260v5270-2022-05-30) [Compare Source](typescript-eslint/typescript-eslint@v5.26.0...v5.27.0) ##### Bug Fixes - **eslint-plugin:** \[no-type-alias] handle Template Literal Types ([#​5092](typescript-eslint/typescript-eslint#5092)) ([8febf11](typescript-eslint/typescript-eslint@8febf11)) ##### Features - \[4.7] support new extensions ([#​5027](typescript-eslint/typescript-eslint#5027)) ([efc147b](typescript-eslint/typescript-eslint@efc147b)) - **eslint-plugin:** \[ban-ts-comment] add descriptionFormat option ([#​5026](typescript-eslint/typescript-eslint#5026)) ([1fb31a4](typescript-eslint/typescript-eslint@1fb31a4)) - **eslint-plugin:** \[no-misused-promises] warn when spreading promises ([#​5053](typescript-eslint/typescript-eslint#5053)) ([61ffa9e](typescript-eslint/typescript-eslint@61ffa9e)) - **eslint-plugin:** \[space-infix-ops] missing error report for conditional types ([#​5041](typescript-eslint/typescript-eslint#5041)) ([0bfab6c](typescript-eslint/typescript-eslint@0bfab6c)) </details> <details> <summary>typescript-eslint/typescript-eslint (@​typescript-eslint/parser)</summary> ### [`v5.27.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#​5270-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5260v5270-2022-05-30) [Compare Source](typescript-eslint/typescript-eslint@v5.26.0...v5.27.0) ##### Bug Fixes - **types:** remove leftovers from removal of useJSXTextNode ([#​5091](typescript-eslint/typescript-eslint#5091)) ([f9c3647](typescript-eslint/typescript-eslint@f9c3647)) ##### Features - \[4.7] support new extensions ([#​5027](typescript-eslint/typescript-eslint#5027)) ([efc147b](typescript-eslint/typescript-eslint@efc147b)) </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1385 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
PR Checklist
@ts-expect-error: TS1234 because xyz
#4253Overview
For maximum flexibility, I didn't go with a hardcoded format, but allow passing in any kind of format.
I personally don't like this API design a lot, so I'd like to get settled on the API before writing docs.