Subject: Re: [PATCH] hook: use correct logical variable

From: Junio C Hamano <[email-masked]>
Date: Tue, 15 Aug 2017 10:28:33 -0700

Kaartic Sivaraam <[email-masked]> writes:

> I guess Junio's suggestion found below seems concise enough
> although it doesn't capture the reason I did the change.

I did shoot for conciseness, but what is a lot more important is to
record what is at the core of the issue.  "I found it by doing A"
can hint to careful readers why doing A leads to an undesirable
behaviour, but when there are other ways to trigger problems that
come from the same cause, "I found it by doing A" is less useful
unless we also record "Doing A reveals the underlying problem X"
that can be shared by other ways B, C, ... to trigger it.  The
careful readers need to guess what the X is.

And once you identify the underlying problem X and record _that_ in
the log message, I and A in "I found it by doing A" becomes much
less interesting and the readers do not have to guess.

Your "A" is 'git commit --amend -s' with the disabled part of hook
enabled.  But I think 'git commit' without "--amend" and "-s" would
also show an issue that come from the same root cause.  The hook
will add SoB that is based on the author, not the committer.  That
resulting commit would be different from 'git commit -s' without the
hook enabled, which would add SoB based on the commiter name (that
would be a "B", that causes a related but different problem that
comes from the same underlying issue "X" which is "we should
consistently use the committer info like other parts of the
system").

In any case, thanks for a fix-up.  Let's move this forward quickly,
as it is an update to a topic that is already in 'master'.

Thanks.