-
Notifications
You must be signed in to change notification settings - Fork 13
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
Possible improvements [config-base] #95
Comments
@developer239 Thanks for all these suggestions! I will look through them in the coming days. Right now I can tell you this much: Stylistic rules and rules where we cannot with a high certainty say that the code is invalid/bad/wrong in any way shall never cause ESLint to fail with non-zero exit code. This was by design from day 1. It's the only reason why we have If you really want to be the evil dictator, you can always run ESLint with And I feel that some other rules mentioned in your Missing rules list are enabled in the ruleset somewhere; are you sure they are missing? 🤔 I will take a look. Some of them look like we should have them. |
@robertrossmann Honestly, I don't mind I just wanted to share my opinion here. 🙂 How I can help right now is I can find rules that we are missing and add them to the config. I created a tool that compares my eslint config with STRV config programmatically. I didn't check all rules myself but I am 99 % sure that rules that I shared here are actually missing in STRV config. |
Great suggestions @developer239! I agree with @robertrossmann that only an invalid/bad/wrong code should fail with a non-zero exit code. Others should be considered as a warning saying something smelly is present. Labeled as
|
Thanks @xhudec for taking your time to review this. 🙂 I pretty much agree with everything you said. Maybe we can keep About the plugins though:
|
Thanks for all the feedback! ❤️ I went through the first section of the rules (Missing rules) and added some here: 471568f. For the ones I did not add, I list my reasoning below. I will try to go through the rest sometime soon. 💪
|
I checked out the other rules where you mention you would increase the severity to For almost all rules there is a possibility that the rule would flag some valid piece of code which would cause a potentially working code to be rejected by CI. This is highly undesireable. I will mention a few rules below where I believe some explanation might be helpful as to why it should not be used/set to
Most of the other rules are cosmetic/stylistic preferences and therefore should be defined as warnings.
Missing plugins: As for Anyway, thank you so much for taking the time to review the ruleset! I incorporated a lot of your proposed changes, even if with a lower severity level. 😇 ❤️ |
Hello 👋
a couple of weeks ago (it feels like a long time now 😅) we discussed what we can do to improve STRV code quality tools. This is my first follow up about
eslint-config-base
.I will share a similar update about
node
,react
,react-native
, andtypescript
this week.Missing rules:
I would highly recommend taking a look at everything labeled as [major]. 🙂
Missing plugins:
Missing rules from missing plugins:
Rules that can be more strict:
no-return-await
I sawasync
keyword being misused, essentially it changes the function to promise for no reason I believe that is a bad practice)Rules that I would change:
error
warn
only for rules that I know I shouldn't break but sometimes I have to (no-console, ban-ts-ignore, camelcase [because of graphql code gen], ...)The text was updated successfully, but these errors were encountered: