git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Stefan Beller <sbeller@google.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Michael Haggerty <mhagger@alum.mit.edu>,
	"git@vger.kernel.org" <git@vger.kernel.org>,
	Jeff King <peff@peff.net>
Subject: Re: [PATCHv3] refs.c: enable large transactions
Date: Fri, 24 Apr 2015 11:12:36 -0700	[thread overview]
Message-ID: <20150424181236.GK5467@google.com> (raw)
In-Reply-To: <CAGZ79kYGDOUgzKmQOLAXkYYb-HZJCw3Y=iSLXWHVXcQ0pAiDBw@mail.gmail.com>

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.

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

> 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,
Jonathan

  parent reply	other threads:[~2015-04-24 18:12 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-22 21:30 [PATCHv3] refs.c: enable large transactions Stefan Beller
2015-04-23 17:56 ` Junio C Hamano
2015-04-24  0:21   ` Stefan Beller
2015-04-24  1:37     ` Junio C Hamano
2015-04-24 16:16       ` Stefan Beller
2015-04-24 17:19         ` Jeff King
2015-04-24 18:12         ` Jonathan Nieder [this message]
2015-04-24 18:31           ` Stefan Beller
2015-04-24 20:17           ` Jeff King
2015-04-25  4:23             ` Junio C Hamano
2015-04-25  5:00               ` Jeff King
2015-04-25  5:24                 ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150424181236.GK5467@google.com \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mhagger@alum.mit.edu \
    --cc=peff@peff.net \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).