-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
feat(eslint-plugin): add require-array-sort-compare rule (fixes #247) #261
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #261 +/- ##
==========================================
+ Coverage 94.6% 94.63% +0.03%
==========================================
Files 65 66 +1
Lines 2778 2794 +16
Branches 720 723 +3
==========================================
+ Hits 2628 2644 +16
Misses 57 57
Partials 93 93
|
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.
requesting changes specifically for the vscode setting.
code otherwise LGTM
Thank you for the review! I updated this PR. |
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.
LGTM
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.
LGTM
Do we want to add an option I'm just thinking - I can see some people getting annoyed at this rule for two reasons:
|
People can use
String#localCompare has some options. I don't oppose the option, but I'm not sure if it's needed. I think, maybe, we can discuss it on another issue. |
Figured it was worth raising just so we all thought about it. I'm more happy to punt on it till someone asks for it :) |
@mysticatea You forgot to add the rule to https://github.com/typescript-eslint/typescript-eslint/tree/master/packages/eslint-plugin#supported-rules |
weird... that should have been caught by the CI. |
we are not running eslint-docs check anymore, we have to update it first (support for scoped packages) and add it to CI |
@armano2 I pushed the update that adds support for scoped packages ( |
Fixes typescript-eslint#144 Requires ~~typescript-eslint#259~~, ~~typescript-eslint#260~~. - added a util to make it standardised and easier to add default config for a rule - configured recommended based on typescript-eslint#144 - purposely switched `recommended` prop to be `"error" | "warning" | false` - inside the eslint repo, it should be `true`. otherwise it's just a property that isn't used officially by eslint. It's truthy so `eslint-docs` still work. - changed recommended generator to accept `"error"`/`"warning"` for more configurability. - adjusted default config of certain rules that didn't match our recommendations.
Fixes #247.
This PR adds
require-array-sort-compare
rule that enforces givingcompare
argument toArray#sort()
method.