Skip to content
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

RFC: Use more common exit code logic #194

Open
l0b0 opened this issue Jul 28, 2021 · 1 comment
Open

RFC: Use more common exit code logic #194

l0b0 opened this issue Jul 28, 2021 · 1 comment

Comments

@l0b0
Copy link
Contributor

l0b0 commented Jul 28, 2021

gitlint has a unique concept of exit codes:

  • If the number of errors is between 0 and 251 the exit code is the number of linting issues (not sure whether this counts the number of messages or there can be more than one linting issue with a single commit message).
  • If the number of errors is higher than 251 the exit code is 252.
  • Exit codes 253, 254 and 255 are used for three other specific types of errors.

This is IMO suboptimal, for several reasons:

  • Any code which tries to check for "is there an error in any of these commit messages" is going to have to be more complicated than necessary, basically checking for an exit code in the range 1 through 251 rather than a single value.
  • It doesn't leave space for any other error types. If any more error types are needed (unknown error, file unreadable, what have you), the API needs to have a breaking change, using one of the previously occupied exit codes for a new purpose.
  • The number of errors doesn't really give any more useful information than "there is an error in one of the checked commit messages", because the offending commit messages are printed verbatim.
  • In basically every other shell tool the exit code signifies a category of errors, which means understanding the gitlint API is a little bit harder than it needs to be.
  • Some exit codes, while in principle completely application-specific, have existing, well-known meanings which conflict with using the entire range. See for example the Bash manual and Are there any standard exit status codes in Linux?.

Would you be willing to consider (not necessarily implement the change) changing this to use a more common scheme? Something like this:

  • 1: Unknown error
  • 2: There are linting violations
  • 3: Usage error
  • 4: Git context error
  • 5: Config error

This shifts the three main existing errors down by 250, for a mnemonic change, and avoids the issues above.

@jorisroovers
Copy link
Owner

jorisroovers commented Feb 10, 2022

Yeah, if I were to re-implement gitlint today and with this extra information, that's probably what I would do. I was trying to be clever, that was dumb. Hindsight is 20/20 🤷‍♂️

Now I guess we need to choose between keeping backward compatibility or supporting the defacto standard. It’s unfortunate that we don’t have any data on how many users actually rely on the specific exit codes.

My gut feeling is that the vast majority of users don’t check for specific exit codes (other than >0). I’m not sure whether the ones that do have a strong preference on this and whether they’d prefer to go through the hassle of changing their wrapper scripts for the sake of following the standard (which has little merit by itself).

Long story short, I have no strong opinions here. Maybe @sigmavirus24 does?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants