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

From: Junio C Hamano <[email-masked]>
Date: Fri, 19 Jun 2015 10:33:50 -0700

Johannes Schindelin <[email-masked]> writes:

> 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.

>>     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.

>> 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.