Re: [PATCH v5 00/19] Introduce an internal API to interact with the fsck machinery

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 19, 2015, 17:33 UTC
Message-ID: <xmqqfv5na9hd.fsf@gitster.dls.corp.google.com>
In-Reply-To: <e3f2c023e59c3608ebbb7e88a6f18d27@www.dscho.org>

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

Show 7 quoted lines

> I basically made up names on the go, based on the messages.
>
>> Some of the questionable groups are:
>> 
>>     BAD_DATE DATE_OVERFLOW
>
> I guess it should be BAD_DATE_OVERFLOW to be more consistent?

I am not sure about "consistency", but surely a common prefix would help readers to group things. But for this particular group, I was wondering if singling out "integer overflow", "zero stuffed timestamp", etc. into such a finer sub-errors of "you have a bad timestamp" was beneficial.

Show 5 quoted lines

>>     BAD_TREE_SHA1 INVALID_OBJECT_SHA1 INVALID_TREE
>> 
>>     BAD_PARENT_SHA1 INVALID_OBJECT_SHA1
>
> So how about s/INVALID_/BAD_/g?

It is not just about distinction between INVAID and BAD.

I was basically wondering what rule decides which one among BAD_TREE_SHA1, INVALID_OBJECT_SHA1 and INVALID_TREE I would get when I have a random non-hexdigit string in various places, e.g. after 'tree ' in the object header of a commit object, after 'tag ' in a tag object that says 'type tree', etc.

Show 5 quoted lines

>> Also it is unclear if NOT_SORTED is to be used ever for any error
>> other than a tree object sorted incorrectly, or if we start noticing
>> a new error that something is not sorted, we will reuse this one.
>
> s/NOT_SORTED/TREE_&/ maybe?

If that error is specific to tree sorting order, then that would be a definite improvement.

Thanks.