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. 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
