On 04/18/2017 02:36 AM, Gregory Szorc wrote:
On Mon, Apr 17, 2017 at 4:10 PM, smaug <sm...@welho.com> wrote:
On 04/17/2017 06:16 PM, Boris Zbarsky wrote:
A quick reminder to patch authors and reviewers.
Changesets should have commit messages. The commit message should
describe not just the "what" of the change but also the "why". This is
especially
true in cases when the "what" is obvious from the diff anyway; for larger
changes it makes sense to have a summary of the "what" in the commit
message.
As a specific example, if your diff is a one-line change that changes a
method call argument from "true" to "false", having a commit message that
says
"change argument to mymethod from true to false" is not very helpful at
all. A good commit message in this situation will at least mention the
meaning for the argument. If that does not make it clear why the change
is being made, the commit message should explain the "why".
Thank you,
Boris
P.S. Yes, this was prompted by a specific changeset I saw. This
changeset had been marked r+, which means neither the patch author not the
reviewer
really thought about this problem.
And reminder, commit messages should *not* be stories about how you ended
up with this particular change. They should just tell "what" and "why".
It seems like using mozreview leads occasionally writing stories (which is
totally fine as a bugzilla comment).
I disagree somewhat. As a reviewer, I often appreciate the extra context if
it helps me - the reviewer - or a future me - an archeologist or patch
author - understand the situation better.
That is why we have links to the bug. Bug should always be the unite of truth
telling
why some change was done. Bugs tend to have so much more context about the
change than any individual commit message can or should have.
If that context prevents the
reviewer or a future patch author from asking "why didn't we do X [and
spending a few hours waiting for a reply or trying to find an answer]" then
that context was justified. I do agree there are frivolous stories that do
little more than entertain (e.g. the first paragraphs of
https://hg.mozilla.org/mozilla-central/rev/6b064f9f6e10) and we should not
be encouraging that style.
Overlong unrelated commit messages just make it harder to read blame.
This is the tail wagging the dog. Please file a bug against the tool for
letting best practices interfere with an important workflow.
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
