On Tuesday, April 18, 2017 at 11:58:11 AM UTC+12, smaug wrote: > 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.
But then some bugs may accumulate hundreds of comments and it becomes unreasonable to expect future maintainers to read through them all, when the commit descriptions could just present a selectively-useful history of the "how we got to this solution". > > > 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
