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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Apr 24, 2015 at 11:12 AM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote:
> Stefan Beller wrote:
>
>> 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.
>
> Lowercase, brief, with no period at the end is the convention.  This
> is user-facing UI, so consistency matters.  It's not author's choice.
>
> Existing examples of capitalized messages fall into a few categories:
>
>  - shell scripts used to tend to use the capitalized form, and when
>    they got rewritten as builtins the messages weren't cleaned up at
>    the same time
>
>  - some patch authors have different habits and it wasn't worth going
>    back and forth to fix it (but the convention still stands, and the
>    result of a program that can't decide how to talk to the user is
>    still harmful)
>
>  - once there are a few examples, they get copy/pasted and imitated
>
>> 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.
>
> This seems like an after-the-fact justification for a mistake.
>
> Often there is a choice about whether the caller or the callee to a
> function prints an error.  If the caller does, it can say more about
> the context.  If the callee does, the message is in one place and can
> be tweaked more easily to be more useful.
>
> To get the benefits of both, we could print a backtrace with every
> error.  That way, the callee can describe the error in one place but
> the context is all there.  But I'm really glad we don't do that!
>
> The main purpose of most error messages is instructional: they give
> information to the user in a way that is abstracted from the
> implementation (the user doesn't care what function it happened in!)
> that tells them what happened and what they can do next.
> Gratuitous inconsistency in formatting doesn't help with that.

I agree we should fix the formatting, but I was pointing out the side effects
and how to avoid the side effects. So what I am proposing is a cleanup of
the warning messages as well as a GIT_TRACE_ERRORS variable as
Jeff proposed, because then we have all the benefits.

If we were to just cleanup the error messages we improve on certain aspects
(UI), while making others worse.


>
> Actually, I wouldn't mind an environment variable that tells error()
> to include a backtrace if someone wants it.  (See backtrace(3)
> for implementation hints if interested in doing that.)

CONFORMING TO
       These functions are GNU extensions.

I guess I can live with backtrace, but the Git for Windows people may
hate me for it.


>
>> Just 3 days ago ("Regular Rebase Failure"). I used different
>> capitalization to get a better understanding where the error may be.
>
> Wouldn't it be better if there weren't so many similar messages
> produced in different contexts in the first place?  (I.e., this
> suggests to me that we should move more in the direction of
> callee-produces-error.)
>
> Sorry, that was a long way to say very little.  I just wanted to
> emphasize that when a UI inconsistency has a useful side effect, while
> that can be useful to understand and learn from, we should not be
> afraid to fix it.  And to clear up any ambiguity about git's error
> message convention. :)
>
> Thanks and hope that helps,

I really appreciate the background information provided!

Thanks,
Stefan

> Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]