IanVS / eslint-nibble

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

Option to just print all errors - use for CI lint enforcements #59

Closed ScriptedAlchemy closed 4 years ago

ScriptedAlchemy commented 4 years ago

I have a large app with various lint errors, I want to apply the auto fix only to a select set of rules. This project does a beautiful job, I’d like to use it in CI to lint a select set of rules- if there’s errors it fails the build. This would let me enforce linting across things we already fixed. I cannot run lint in CI due to number of errors.

ScriptedAlchemy commented 4 years ago

@IanVS

IanVS commented 4 years ago

Just to make sure I understand correctly, it sounds like what you want is to use https://github.com/IanVS/eslint-filtered-fix, but you want the exit code to be non-zero if there are any linting errors remaining after the fixes, so that CI would fail? Or, are you looking for something that doesn't do any fixes, and just checks for linting errors using your project's config, but filtered just to a subset of rules that you specify?

ScriptedAlchemy commented 4 years ago

Just checks for a subset of errors and either fails of passes.

Preset much I have lots of lint issue on some projects. I use your tool to fix one rule at a time but I want to also enforce the ones already fixed. Because there’s so many errors I can’t lint in CI otherwise the build just fails. So I was hoping to use something like your tool to verify there’s no linting errors on a subset of the rules to make sure that issues already fixed are not reintroduced.

Pretty much this first example you gave “ Just to make sure I understand correctly, it sounds like what you want is to use https://github.com/IanVS/eslint-filtered-fix, but you want the exit code to be non-zero if there are any linting errors remaining after the fixes, so that CI would fail”

IanVS commented 4 years ago

Okay I understand. I don't think anything I have will meet your needs right now, but I'll take a crack at putting something together.

ScriptedAlchemy commented 4 years ago

Appreciate it! Would you accept a PR to this project if I took a stab at it, or what repo would be most suitable? Any pointers on how you’d like to see implementation? It seems like it could be relatively trivial.

Basically, I’d extend your CLI options and have it run whatever it does now. If there’s a —ci flag then instead of showing any prompts I’d have it either echo out the stdout from eslint binary with the appropriate exit code. Granted I’ve not taken a look at the source code but I’d rather invest in an existing tool than fork

IanVS commented 4 years ago

I need to give this a little thought. My first reaction is that this is a different enough use case that it should be separate package from eslint-nibble. However, maybe it's also reasonable that others might be in your situation and want to set up some protection to ensure new linting errors are not introduced while working down existing errors.

I'm leaning towards breaking out eslint-filtered-fix into a new eslint-filtered package that eslint-nibble can also use. What would you think about using a CI=true environment variable instead of an explicit flag. Many CI environments set that flag automatically, and running eslint-nibble in interactive mode in CI is pretty pointless.

Have you done any searching to see if there are already any tools out there to run ESLint on a subset of rules, while still using a project's eslint config?

IanVS commented 4 years ago

@ljharb do you have any thoughts on this issue of running eslint on a subset of rules for CI?

ljharb commented 4 years ago

Let’s see.

A separate package makes sense, there’s lots of use cases for filtering eslint rules.

Using the CI env var (of which there are many; use the popular npm package for this whose name i forget) is great, but an explicit flag is a must - implicitness is sketchy enough without an explicit way to set or disable something. Separately, there’s other cases that aren’t CI where interactive wouldn’t make sense, like when in a non-tty terminal (a pipe, E.g.).

ScriptedAlchemy commented 4 years ago

I searched and searched, your work was basically the only decent option out there.

I do like the idea of an eslint-filtered type package. I’ll mess around with additional functionality next week. Filtered fix is pretty clean and simple so I think it could be pretty straightforward

I’ll do another round of searching before forking, if you’re planning on a eslint-filtered repo, I can pr back to that otherwise i can create the repo and add you as a maintainer if you find a use

IanVS commented 4 years ago

Alright, after re-familiarizing myself with how this package is put together, I'm going to add the flag directly here, and maybe later also create a separate standalone package. Should have a PR up for you to take a look at shortly, @ScriptedAlchemy. Thanks for bearing with me here. Busy time of the year. ;)

IanVS commented 4 years ago

I've published a beta version with --no-interactive and --format flags. Can you try it out using eslint-nibble@next and let me know if it works for you, @ScriptedAlchemy?

IanVS commented 4 years ago

@ScriptedAlchemy do you have any feedback on the beta build? If it meets your needs, I'd like to promote it to stable.