feat(severity): color warning severity, non-zero exit when issue != default severity

[jaredallard]

What this PR does: This PR adds a new print-severity field that toggles printing severity information on the text printer. By default, we now change the color of the error if the severity is set to warning. Also changed is when issues are discovered, we only return a non-zero exit code if it's equal to the default severity (e.g. error). If it's a non-default severity, e.g. warning, we return a zero exit code.

Imporant Note: I know this is super opinionated, so let me know if there's a different direction we want to take this. I'm not attached to this implementation at all! Also happy to split this into multiple PRs 😄

[boring-cyborg]

Hey, thank you for opening your first Pull Request !

[CLAassistant]

All committers have signed the CLA.

[jaredallard]

I don't really like this print-severity field in general, but I want to try to maintain older behaviour if possible. I was thinking of adding something like exit-non-zero-severity to replace the default severity field usage in this PR. Still need to look at this codebase more to figure out how that's implemented.

[jaredallard]

The only time I really found this useful is if coloring is turned off, so I'm happy to remove it / restructure this.

[jaredallard]

I'm sure this isn't great because this seems to attach to specific severity levels -- which I don't see well defined in this repository. Let me know if we want to define coloring a different way, if at all. I like this, personally.

[ldez]

Hello,

I have a problem with your PR because it contains 2 topics:

• color on the severity
• the exit code depends on the severity

It's 2 different topics, so it means 2 different PRs/issues.

Regarding the exit code, as far as I remember, discussions have already taken place on this subject and overall the decision was made not to correlate severity and exit code.
So I recommend looking for those discussions, and if you don't find them, you can open an issue to handle this topic.

Related to:

• Coloring Options for Severity #1839
• golangci-lint cli returning non-zero value for issues raised at a severity level not of error #1981

[butuzov]

Hi, I did some related work in the past, so cannot really review this code as I am biased. I have tested both (actually one) features on myself and the company I was working for at that moment. I am also not aware of the discussion on exit codes and did my own research. However, I can give a small intro on the topic:

Both severity and coloring are connected, but you better approach it from the point of the severity of exit codes first.

• You either fail execution or not (exit with 1 or 0).
• golangci-lint option issues-exit-code needs to have precedence, to have backward compatibility.
• You can start incorporating severity from the point it's implemented in golangci-lint see severity configuration.

What I found after using this on my own flow:

• No need for extra colors, red for errors, and a custom one for warnings is pretty much top of what people use.
• No need for extra severity types (like info or debug). warnings and errors are what may be the top of what is needed.

Problems:

• Should custom severity hold extra exit code? complicated questions - as what should we do if there are three different types of issues?
• Customization: currently it's supported only for the linters, and you can define whatever severity you want. You can't set up severity per issue (like for example for exclude-rules). Some of the linters provide their own severity (like revive) - what should we do in this case ? Define rule twice? Follow original rules and break backward compatibility?

IMO, this feature is a bit controversial. I did implement that for myself, I didn't like that it gives a bit more space than needed. And tbh I didn't look for the answers from the community. The support for the idea itself was low at(hat)m, I am not even talking about implementation and support.

[ldez]

As suggested by my previous comment the PR cannot be merged like this, you have to split your work.

I will close the PR.