Re: [PATCHv3] refs.c: enable large transactions

From: Stefan Beller <sbeller@google.com>
Date: Apr 24, 2015, 16:16 UTC
Message-ID: <CAGZ79kYGDOUgzKmQOLAXkYYb-HZJCw3Y=iSLXWHVXcQ0pAiDBw@mail.gmail.com>
In-Reply-To: <xmqq8udi2tn8.fsf@gitster.dls.corp.google.com>

On Thu, Apr 23, 2015 at 6:37 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 22 quoted lines

> Stefan Beller <sbeller@google.com> writes:
>
>> On Thu, Apr 23, 2015 at 10:56 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>>>> +             int save_errno = errno;
>>>> +             error("Couldn't reopen %s", lock->lk->filename.buf);
>>>
>>> No need to change this line, but I noticed that we might want to do
>>> something about the first one of the following two:
>>
>> I personally like to have each error(...) to have a unique string, such
>> that when you run into trouble (most likely reported by a user),
>> you can easily pinpoint where the exact error is.
>
> I was hoping that the "grep" patterns were strong enough hint, but
> let me be explicit.  I was commenting on "Could" not being spelled
> as "could".
>
>>>     $ git grep -e '[    ]error(_*"[A-Z]' | wc -l
>>>     146
>>>     $ git grep -e '[    ]error(_*"[a-z]' | wc -l
>>>     390

I understood that you were talking about Could being capitalized. Though it's distributed 30/70 which hints to me we do not care in enough to explain the capitalized letters as slip-throughs on review or such, but it looks like the authors choice to me.

I think it's a mistake to s/Could/could/g for all errors in the code base as it reduces the information provided in the error messages. Just 3 days ago ("Regular Rebase Failure"). I used different capitalization to get a better understanding where the error may be.

So if we throw away that information, we should add new information to make the spot of the error easily findable in the source. That's why I proposed the idea of the version,filename,linenumber as that is one of the strongest signals (most information in a short amount of text) I can imagine.