Re: [PATCH 04/18] Offer a function to demote fsck errors to warnings

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 22, 2014, 22:40 UTC
Message-ID: <xmqqy4pz71g7.fsf@gitster.dls.corp.google.com>
In-Reply-To: <alpine.DEB.1.00.1412222330080.21312@s15462909.onlinehome-server.info>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 24 quoted lines

> Hi Junio,
>
> On Mon, 22 Dec 2014, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> 
>> >> In other words, at some point wouldn't we be better off with
>> >> something like this
>> >> 
>> >> 	struct {
>> >>         	enum id;
>> >>                 const char *id_string;
>> >>                 enum error_level { FSCK_PASS, FSCK_WARN, FSCK_ERROR };
>> >> 	} possible_fsck_errors[];
>> >
>> > I considered that, and Michael Haggerty also suggested that in a private
>> > mail. However, I find that there is a clear hierarchy in the default
>> > messages: fatal errors, errors, warnings and infos.
>> 
>> I am glad I am not alone ;-)
>> ...
> Oh, but please understand that this hierarchy only applies to the default
> settings. All of these settings can be overridden individually – and the
> first override will initialize a full array with the default settings.

But that means that the runtime needs to switch between two code with and without override, no?

> 	if (options->strict_mode)
> 		return options->strict_mode[msg_id];

In other words, I think this is misleading and unnecessary optimization for the "full array" allocation. A code that uses an array of a struct like the above that Michael and I independently suggested would initialize once with or without an override and then at the runtime there is no "if the array is there use it" conditional.

I do not know why Michael suggested the same thing, but the reason why I prefer that arrangement is because I think it would be easier to read and maintain.

Thanks.