Thanks, looks much better, especially with S-o-b: line.
I do not get the funny punctuation, sorry.
Anyway, here is a bit more detailed reason behind my request.
(1) The subject is primarily to help people who look at
shortlog (or "gitk") to get the overview of recent changes,
or in general "changes within a given range".
Readers are most interested in what areas are affected
(e.g. the command from the end-user's point of view, or the
internal implementation) and what the nature of the change
was (e.g. bugfix vs enhancement). To help them, the
Subject: line summarizes "what the change is about".
Your Subject: line is _perfect_. It identifies the area as
"git-commit" instead of "builtin-commit.c", because it is
not about fixing internal implementation of that file, but
about the end-user experience interacting with the command.
It also makes it clear that it is a fix by saying that we
failed to exit with non-zero status code upon some failure.
(2) The body of the commit log message is primarily to help
people who look at this particular commit 6 months down the
road to see why things got there that way.
Reason behind the logic in the code _after_ the change can
be left in in-code comments. The reason behind the change
itself (why the logic behind the code _before_ the change
was faulty or insufficient, and the logic behind the new
code is better) is not captured well in such a comment (and
we do not want to clutter the code comments with a long "in
ancient versions we used to do this but then we updated it
to do that but now we do it this way instead." --- I made
that mistake earlier and I suspect some of the older source
files still have them).
The commit log message should describe why the change was
needed (e.g. "The earlier code assumed X because it knew Y
won't happen, but that is not the case anymore since commit
Z, so this code stops relying on that assumption and
implements the logic this way instead"), why the proposed
implementation was thought to be the best one to choose
(e.g. "We alternatively could do W and it may have some
performance edge, but this way the code is simpler and in
my benchmark with real life data I did not see significant
gain from the added complexity").
How the code was changed in this commit does not need to be
described; that can be seen in "git show $this_commit"
output easily.
In this particular case, I think it is probably sufficient to
briefly describe what "failure to commit the index", mentioned
in the summary line, means. For more complex fixes and
enhancements, it would make a good log message to also describe
what the plausible cases the updated codepath is triggered, from
the point of view of the committer/author when making the commit
(IOW, what scenarios the updated behaviour intends to handle),
like you did in this version.
Such a description will help the person who finds the change was
faulty or insufficient 6 months down the road by allowing him to
say "Aha, the change considered these cases but forgot to
consider this case, and missed the fact that this part of the
code needs to work differently in that particular case" while
making further fixes. Otherwise the person will be left
wondering if the omission of handing that case he encountered
was deliberate or a simple oversight. With the comment, he can
make his fix with more confidence.
In addition, at the end of the body, there is expected to be
your S-o-b: line, so it will never be "1-liner".
In any case, thanks for a fix. Will apply.
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
