eslint: Separate exit code for error

I would be against doing that. Introducing an option now just to remove it in a few month doesn’t seem like a good idea. I understand @Soullivaneuh desire to have a working solution now, but it feels like it’s a lot more work for us to do that (introduce a new option, retire a new option, move the code around, etc.).

Traditionally, we have decided against multiple exit codes because there hasn’t been a lot of evidence that multiple exit codes provide enough value to risk breaking existing integrations that might have logic looking only for exit code 1 (like if (exitCode != 1)). My opinion has generally been the same as Ilya’s in that if something the lint fails for any reason, then you need to look at it, so it only matters that the exit code isn’t zero.

As a reminder, we had an ill-fated experiment where we (actually me) changed the exit code to be the number of lint errors. We had to roll that back because it broke a lot of things, especially when the number of problems was > 254. That’s part of why we’ve been so reluctant to change exit codes since then.

That said, I can see an argument for making this change as the output for limiting problems vs. a crash is different, and I can see it being helpful for tools to be able to know how to bubble up the error details.

So I’m 👍 for keeping exit code 1 for linting errors and introducing exit code 2 for any other type of errors. That would be a breaking change and would have to go in on a major release. I wouldn’t want separate exit codes for every type of error condition as I think that’s overkill.

I think there are three types of error scenarios here:

1. ESLint runs successfully, and reports linting problems.
2. ESLint fails to run due to user error (e.g. a missing configuration file)
3. ESLint fails to run due to an unexpected internal condition (e.g. a bug in a rule that causes an error to be thrown).

Since you gave a missing configuration file as an example, I’m assuming you are using “When eslint is failing” to refer to category 2. I’m saying that it might be worth distinguishing between expected failures caused by user error, and unexpected failures caused by potential bugs.

Well, suppose there’s some custom handling for actually reporting the CI error. Maybe that would depend on an exit code for whether to message the developer about their code being bad, or the IT group about an environment issue. Much easier to do that by reading an exit code (the only other option at that point is to read stdout/stderr). Hope that helps!

On Oct 6, 2017 10:44 PM, “Ilya Volodin” notifications@github.com wrote:

@platinumazure https://github.com/platinumazure I wasn’t referring to the pain of switching exit codes, I’m just not sure I understand the reasoning behind it. A build failure is a build failure and somebody should look at it. To me, it really doesn’t matter if the build failed due to code issue, or due to config missing. But maybe it’s just me, not sure.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/eslint/eslint/issues/9384#issuecomment-334907891, or mute the thread https://github.com/notifications/unsubscribe-auth/AARWeufqlg6tJyqMg14fwwmT9jGgFHpLks5spvO4gaJpZM4Psqzs .

I’m 👍 for having as many exit codes as we need for different scenarios. And I think it would be good to clarify when changing or adding an exit code should be semver-minor vs semver-major and allow ourselves to make changes when we really need to.

@ilyavolodin You note that CI is usually set up once and then forgotten. By the same token, if we made a change to exit codes (hopefully with semver-major release), CI would only need to be tweaked once, and then forgotten once again. I think this team overestimates the pain of dealing with exit code changes (as long as we don’t do so frequently and we do so under the correct type of semver release).