-
-
Notifications
You must be signed in to change notification settings - Fork 411
feat: add 'unopinionated' config #2715
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
base: main
Are you sure you want to change the base?
feat: add 'unopinionated' config #2715
Conversation
I don't think "Type safety makes this much less useful" is a good reason to make rule 'opinionated'. If I understand it correctly. It should only include rules for correctness, there should be only a few rules. |
45b069b
to
b31c759
Compare
Rules like Strictly speaking, part of |
Ok I think I adjusted for that? |
@@ -1,6 +1,6 @@ | |||
# Enforce a specific parameter name in catch clauses | |||
|
|||
💼 This rule is enabled in the ✅ `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#recommended-config). | |||
💼🚫 This rule is enabled in the ✅ `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#recommended-config). This rule is _disabled_ in the ☑️ `unopinionated` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#recommended-config). |
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.
I think this rule is opinionated.
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.
I'm not following, it's disabled in the unopinionated config. I didn't mark this rule as opinionated. Is this a point of agreement, or...?
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.
Sorry, I thought it's enabled...
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.
I'll take another look.
I didn't go through all rules, but I still think there are too many rules in this preset. |
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, thanks!
Can you fix the conflicts? |
Fixes #896.
Adds all rules from
recommended
to a newunopinionated
config, with the exception of rules that add in subjective opinions >1% of users might not want (per my vague guesstimation). This list was generated by looking through #896 & linked issues, then running on my personal repos:.toMatchInlineSnapshot()
This
unopinionated
config does include the following rules that were mentioned as being candidates for removal:consistent-return
doesn't enforce correctnessI recognize I'm not an expert on this plugin and look forward to learning why some rules should(n't) be on those lists.☺️