Re: [PATCH v2 06/10] fsck.h: move FSCK_{FATAL,INFO,ERROR,WARN,IGNORE} into an enum

> On Thu, Feb 18, 2021 at 11:58:36AM +0100, Ævar Arnfjörð Bjarmason wrote:
>
>> Move the FSCK_{FATAL,INFO,ERROR,WARN,IGNORE} defines into a new
>> fsck_msg_type enum.
>
> Makes sense. As with my previous comment, I wonder if "severity" is a
> more descriptive term.
>
>> diff --git a/fsck.h b/fsck.h
>> index 0c75789d219..c77e8ddf10b 100644
>> --- a/fsck.h
>> +++ b/fsck.h
>> @@ -3,10 +3,13 @@
>>  
>>  #include "oidset.h"
>>  
>> -#define FSCK_ERROR 1
>> -#define FSCK_WARN 2
>> -#define FSCK_IGNORE 3
>> -
>> +enum fsck_msg_type {
>> +	FSCK_INFO = -2,
>> +	FSCK_FATAL = -1,
>> +	FSCK_ERROR = 1,
>> +	FSCK_WARN,
>> +	FSCK_IGNORE
>> +};
>
> You kept the values the same as they were before, which is good in a
> refactoring step, but...wow, the ordering is weird and confusing.
>
> In FATAL/ERROR/WARN/IGNORE the number increases as severity decreases.
> Maybe reversed from how I'd do it, but at least the order makes sense.
> But somehow INFO is on the far side of FATAL?
>
> Again, not something to address in this patch, but I hope something we
> could maybe deal with in the longer term (perhaps along with fixing the
> weird "INFO is a warning from the user's perspective, but WARNING is
> generally an error" behavior).
>
> I also know that this is assigning WARN and IGNORE based on
> counting-by-one from ERROR, so it's correct. But I think it would be
> more obvious if you simply filled in the values manually, so a reader
> does not have to wonder why some are assigned and some are not.