Change threshold definition in detekt

[arturbosch]

As mentioned by @schalkms in #300 detekt's definition of a threshold is: Every value which is greater than the threshold should be flagged eg. for threshold 3:

• value 3 will not get flagged as 3 is not > 3
• value 4 will get flagged as 4 > 3

Other tools uses a value-inclusive-threshold: Every value which greater than or equals the threshold should get flagged eg. for threshold 3:

• value 3 and 4 get flagged as 3|4 >= 3

Discuss and vote :)

[arturbosch]

Leave the threshold as it is now. Every value > threshold get flagged.

[arturbosch]

Change to every value >= threshold get flagged.

[vanniktech]

I have no preference since even having one warning / error fails my builds. Naturally if the threshold is 3, I'd expect to only fail on 4.

If we look at the definition:

the magnitude or intensity that must be exceeded for a certain reaction, phenomenon, result, or condition to occur or be manifested.

It uses the word exceeded which would mean 3/3 okay 4/3 not.

[schalkms]

>=
for me

[sschuberth]

For me the most natural semantics (independently of whether you call it "threshold" or something else) would be to specify "the maximum number of accepted defects". This is nice because in a project where you continuously reduce the number of defects, the current threshold would show the remaining number of defects to address. I believe this is more natural than specifying the number of defects starting from which a build is supposed to fail.

So, how about a compromise and keep the failThreshold keyword, but also introduce a maxDefects keyword, and not allow to specify both?

[arturbosch]

First: I will change the threshold definition to value >= threshold like it is used in other tools like pmd

Second: Remove warningThreshold and failThreshold and introduce maxDefects. What do you think about this? Anyone ever used this for warnings?

@vanniktech @schalkms @Mauin @sschuberth

[Mauin]

Sounds good. I haven't used either thresholds much so don't have a strong opinion on the warning/error thresholds overall.

[arturbosch]

I think this is done for now :).

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related topics.