Re: log messages > comments

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 4, 2022, 19:41 UTC
Message-ID: <xmqqk0d91p5s.fsf@gitster.g>
In-Reply-To: <220304.86ilsuf1e8.gmgdl@evledraar.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Show 18 quoted lines

> On Thu, Mar 03 2022, Junio C Hamano wrote:
>
>> Emily Shaffer <emilyshaffer@google.com> writes:
>>
>>>> +The goal of your log message is to convey the _why_ behind your
>>>> +change to help future developers.
>>>> +
>>>
>>> This is pretty compelling. Is it clear enough why we care about this in
>>> the commit message, as opposed to in the patch description (cover letter
>>> or post-"---" blurb)? Is it too obvious to explicitly mention that the
>>> commit message is the first thing we try to make sense of during a 'git
>>> blame' or 'git bisect'?
>>
>> Again, patches welcome ;-)
>
> I think for something that's a stylistic preference I'd see why Emily
> would try to see how you feel about it first.

I do not think there is a need to write down stylistic preference. I may show my preference during my reviews, of course, but I won't take preference-only things as a blocker.

I also do not think things like "'We used to do X here but we do Y because ...' does not belong to in-code comment, but to log message" is "stylistic preference", and if people are unclear about, I agree that we should spell it out.

An example, I can think of offhand, of what should be in comment, whether we also write in the log, is "We do X here because that other code expects us to", when it is tricky to figure out by reading the code by itself without going back to "git blame".

"git blame" certainly can be used to figure out which commit touched the line that does X (which is hard to figure out why), and the log message can refer to "that other code expects us to", but that is an extra operation.

Also, when we really need to figure out, it is wonderful that we can ask "blame" to give us the commit, and can look at "that other code" in the same commit by checking it out to the working tree, especially when "that other code" may have drifted and the original reasoning no longer applies (iow, what we find out from "git blame" may become stale, and it will stay stale forever because we cannot rewrite the history that old).

Now, it is certainly not black/white decision to say what is and what is not tricky to figure out in the code. We shouldn't be commenting obvious things. Two yardsticks I use are

 (1) if reviewers raise questions during a review, it may indicate
     that it is worth commenting.

 (2) if an earlier round of the same series had a bug around the
     area, it may indicate that the fixed code is worth commenting.

but the way I use them is more to say "I found this uncommented code somewhat tricky---but nobody asked a question and it has stayed the same from the initial round, so it may be clear enough for other people, and after all I managed to figure out myself, so it probably is OK to leave it uncommented".