Re: [PATCH 3/3] commit: Show the committer ident when is different from the parent

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> On Thu, May 01, 2008 at 10:34:44AM +0200, Santi Béjar wrote:
>
>> >  > 1) the committer ident is different from the parent
>> >  > 2) the committer ident is set automatically
>> >
>> >  Honestly, I think just "2)" is probably fine (where automatically
>> >  presumably means "from GECOS"). I see what you are trying to accomplish
>> >  with "1)", but it's so workflow specific as to be useless.
>> 
>> I don't see "1) as workflow specific, it is there to minimize the
>> number of time it is shown, ie somebody might prefer the automatic
>> ident, so as long as it does not change it is not shown.
>
> Yes, but whether it actually succeeds in minimizing is dependent on the
> workflow of the user. I.e., it works great if I tend to build on small,
> personal repositories (or topic branches), or if I tend to receive other
> people's work by patches that I commit myself. But if I am pulling from
> somebody else's repo (as many of us do from git.git), or if I use a
> shared repo with the other committers, then I am very likely to be
> building on a parent that was committed by somebody else (and
> consequently will see the committer mentioned way more often than was
> intended).

The goal of the patch as I understand it is to prevent mistakes of
committing under a wrong committer ident, isn't it?  Why does "minimizing"
come into the picture?

Once you have a good algorithm to see when to trigger the warning that the
user might be using an unintended committer identity, I do not think you
should refrain from issuing the warning when you see the offending
committer ident and whose commit you are building on top of should not
affect it.  Otherwise, the user will get the warning once (or not even get
the warning because the commit was made with a "commit -s -m 'typofix'"
one liner), and keep using the wrong ident without noticing before
building up a long chain of commits.  If the identification algorithm is
bogus, that would result in too many false hits to be annoying, and in
that case, (1) I would not apply such a bogus algorithm until it gets
into reasonable shape, (2) we would improve it after it gets applied and
if still makes noise on a hopefully rarer false positive cases, and/or (3)
the warning will be accompanied with a suggestion to explicitly use
user.name and/or user.email in the configuration file to allow the user to
squelch it.

I think the other patch about showing the author when you are committing
other's changes is a good move, by the way.