search

IanVS / eslint-nibble

Ease into ESLint, by fixing one rule at a time
MIT License
787 stars 29 forks source link

Line-ignore feature request #39

Closed brokentone closed 6 years ago

brokentone commented 7 years ago

First off -- awesome project! We've been using this at Condé Nast to help with summaries of lint rule impact, as well as helping us roll out the changes / fixes.

That said, we've found that in general, lint, especially at company scale, is a challenging thing to work with. Providing best practice rules, while being realistic about the downstream impact to your consumers is tricky. Tools like this certainly help us find our way forward, however sometimes it takes multiple strategies.

Right now our flow looks like:

  1. Choose new rule or ruleset (or upgrade a base package).
  2. Fix all offending lines of code to get back to 0 errors and ~0 warnings.
  3. Disable any rules which require too much effort to correct, in repo-specific file.

When point 3 occurs, then we lose the entire benefit of that rule, people likely to continue to write the non-desired code.

Rather than disabling a rule, it would be better to ignore the specific infractions of the current codebase, and only apply moving forward. This would ensure that new code at least follows the new standard.

In my mind, this could be accomplished by adding a feature to this tool. After choosing the rule to work on, a user could be presented with an "ignore lines" option in addition to the "fix" option. This would go insert line-level eslint ignore statements. I understand this is a little afield of what the tool currently does, but I don't see any other tools in the space able to help.

I would be interested in helping out, if this is a desired feature.

IanVS commented 7 years ago

Hi @brokentone, thanks for the feedback and suggestion. I'm glad eslint-nibble is helping y'all out.

I'm going to have to give some thought to adding eslint-ignore-next-line statements as you suggest. I don't think it belongs within eslint-nibble, but perhaps it could be a separate project. It might even be possible to create it as a meta-plugin and use the eslint "autofixing" to add the disable comments. I'll investigate a bit.

In the meantime, have you looked at the max-warnings cli argument of eslint? It might at least get you part of the way there, by setting the troublesome rule to a warning, and then limiting the number of acceptable warnings so that if more of the problematic pattern is used, linting will fail. I know it's not quite as fool-proof as adding in specific disable comments, but maybe it helps.

IanVS commented 6 years ago

I'm sure you've probably forgotten all about this issue, but I'm going to close it out to clean up the issue list. I don't think this is something we can support, and I'm guessing you've moved on anyway. :)

brokentone commented 6 years ago

@IanVS I appreciate your feedback from some time ago -- I'm still interested in this concept, but I agree at this point that it is not / should not be the concern of your utility. I think, were it interesting to accept the current state of code while ensuring modified/added lines are improved. Leaving comments inline certainly is heavy handed. The better thing might be for eslint itself to have some kind of "lockfile" concept which could "accept" a state of things... anywho, thanks for your thoughts on this!