A reminder about commit messages: they should be useful

[Boris Zbarsky]

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.

[David Major]

I'd like to add to this a reminder that commit messages should describe
the _change_ and not the _symptom_. In other words, "Bug XYZ: Crash at
Foo::Bar" is not a good summary.

This is implied by what Boris said, but I've seen enough of these on my
pulsebot backscroll that it's worth mentioning explicitly.

Thanks!

> _______________________________________________
> dev-platform mailing list
> dev-pl...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform

[James Graham]

On 17/04/17 16:41, David Major wrote:
> I'd like to add to this a reminder that commit messages should describe
> the _change_ and not the _symptom_. In other words, "Bug XYZ: Crash at
> Foo::Bar" is not a good summary.

An unfortunate pattern I see is non-descriptive commit messages for
tests, which is particularly problematic for web-platform-tests that are
later upstreamed. A commit message like "Bug 1234 - Part 2: Tests" is
not totally terrible in the context of Mozilla Central — although
obviously more detail about what is, and especially what is not, covered
would be appreciated — but "Part 2: Tests" is utterly meaningless upstream.

When adding web-platform-tests it would be really appreciated if people
would consider how the commit message will read outside the m-c
repository, and at least mention which features the tests are intending
to cover. Ideally we would also avoid bugzilla review cruft like part
numbers, but I understand that a more realistic solution there is for
the tooling to strip out more of this when upstreaming.

[Gregory Szorc]

A litmus test I apply when writing commit messages is "can the reviewer [or
any subsequent reader of the changeset] discern enough context from the
commit message alone to conduct review or understand the reason for the
change?" If yes, it is a good commit message. If no, it may* not be a good
commit message. I think it was Richard Newman who first told me this
analogy: commit messages are like fragments of a larger picture - if you
combine all the commit messages together, you construct a story for the
history of the code as if you were reading a good book. A bunch of
fragments like "Bug 123456 - Fix foo" don't construct a meaningful story
and are therefore not very useful.

An explicit goal of commit messages I write is that the reviewer/reader
doesn't need to read anything other than the changeset (like Bugzilla
comments) to conduct review or understand the changeset's context. That's
not to say they won't read those things and/or verify that content of the
commit message is true. But in cases where the reviewer trusts the author,
the words they've written, and agrees with the premise of the commit
message, it can save a ton of the reviewer's time. Also, when I as a
reviewer see a detailed commit message, I can assess the comprehension the
author has on the subject and this helps me vary scrutiny to different
aspects of the change, as appropriate. I can also interpret the commit
message as a thesis statement or hypothesis and then gauge whether the code
changes actually do that. If the commit message and code aren't in
agreement, it's a good sign extra scrutiny is needed. From my experience,
the extra context in commit messages has prevented tons of bugs from making
it past review.

* Sometimes there's just too much context and you need to reference things
external to the changeset.

[smaug]

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).
Overlong unrelated commit messages just make it harder to read blame.


-Olli

[Gregory Szorc]

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.

[smaug]

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.

[gsqu...@mozilla.com]

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

[smaug]

I've yet to see that to happen. What is crucial is fast way to browse through the blame in time. So commit messages should be short and descriptive.
Telling what and why. (I very often need to go back to CVS era code). I won't spend time reading several paragraphs of commit messages. Those are just
too long.
Basically first line should tell the essentials 'what' and maybe the most obvious 'why' and the next couple of lines explaining 'why' more precisely.
Don't write stories which will make blame-browsing slower.

[Gregory Szorc]

Exactly. As a reviewer, when I see an empty commit message and load the bug
to get more context and see a wall of text, my reaction is to utter an
expletive. But if I see a descriptive commit message and I trust what's
written there, I can avoid loading the bug and just get on with reviewing.
In other words, that commit message just saved me a bunch of time and got
you feedback sooner all without upsetting me. And generally speaking, we
want to optimize for reducing time spent on review because reviewers are a
more limited (and therefore more valuable) resource than patch authors.

Of course, you can't prevent all bug scouring. But I argue it shouldn't be
necessary in the common case. Furthermore, I would expect a good commit
message to direct the reviewer to the bug if it contains useful context.
Along that vein, perhaps we need a way to "flag" a bug/comment so the
reviewing interface can tell any future reviewer "see bug / comment for
important context." That way we don't have to put so much trust in commit
messages (which can have omissions, often unintentionally, possibly
maliciously).

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

[Nicholas Nethercote]

On Tue, Apr 18, 2017 at 9:58 AM, smaug <sm...@welho.com> wrote:

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

With all due respect, I think you have a different view on this to at least
some people (and perhaps most people).

In a recent similar dev-platform thread you said the following:

"In practice I tend to just read the first line of a commit message and the
most important part there is the bug number."

I was genuinely surprised to read that. Like bz and gps, I strongly prefer
getting context from commit messages than from bug reports. I appreciate
informative commit messages. I put effort into writing them myself, and I
would expect a reviewer to read them given that they are intended to make
their life easier. Obviously, conciseness is also desired, but I prefer
commit messages that err on the side of too much detail rather than not
enough.

(In general, IME the two main ways a patch author can help a reviewer are
(a) write small patches, and (b) write good commit messages.)

Nick

[Ben Kelly]

On Mon, Apr 17, 2017 at 9:21 PM, Nicholas Nethercote <n.neth...@gmail.com
> wrote:

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

> With all due respect, I think you have a different view on this to at least
> some people (and perhaps most people).
>

FWIW I agree with Olli. I look for a good one line summary of the change,
but beyond that I find you really do need to look at the bug to get the
full context.

I don't object to people writing longer commit messages, but that
information needs to be in the bug. Our tools today (splinter and
mozreview) don't do that automatically AFAIK. I think there is an hg
extension you can get to do it with splinter.

I find it much more useful to have an explanatory comment in the bug since
I can see it in context with everything else, links, etc. Its also clear
what version of the patch its associated with. Commit messages often are
not updated with changes to the patch and can be slightly wrong.

Finally, as has been mentioned in previous threads, any real nuance should
be in the patch itself as comments.

Both bugs and code comments are searchable and more easily discoverable.
The tools for finding information in blame are much harder to use. I
greatly prefer bug and code comments.

Just my two cents. (I wasn't going to chime in here, but felt compelled
since Olli's view was being cast as rare.)

Ben

[Jim Blandy]

It seems like there is actually not a consensus on this. (I had thought
Smaug's view was the consensus, and found bz's post surprising.)

Both approaches have tradeoffs. There's a good reason we require the bug
number front and center in a commit message. But I dare you to read the Web
Replay bug <https://bugzilla.mozilla.org/show_bug.cgi?id=1207696> for
context; it's currently on comment 366. (Granted, that bug should have been
subdivided, but there are other examples.)

If this is worth settling, we should definitely *not* settle it on this
list. A mailing list is fine for making one's case or pointing out issues,
but the final decision should be made by a small group of engineering
leadership, and then documented on wiki.m.o. (If it isn't there already! I
looked but couldn't find anything definitive...)

[Nicholas Nethercote]

On Tue, Apr 18, 2017 at 11:34 AM, Ben Kelly <bke...@mozilla.com> wrote:

> FWIW I agree with Olli. I look for a good one line summary of the change,
> but beyond that I find you really do need to look at the bug to get the
> full context.
>

Huh, interesting. Thanks for the data point.

> I don't object to people writing longer commit messages, but that
> information needs to be in the bug. Our tools today (splinter and
> mozreview) don't do that automatically AFAIK. I think there is an hg
> extension you can get to do it with splinter.
>

Yes, 'hg bzexport' defaults to copying the commit message into a bug
comment.

Nick

[Boris Zbarsky]

On 4/17/17 10:45 PM, Jim Blandy wrote:
> It seems like there is actually not a consensus on this. (I had thought
> Smaug's view was the consensus, and found bz's post surprising.)

Really? I know where Olli is coming from, but even in his view a commit
message like the one I was talking about is not OK, I'm pretty sure.

And note that in this case the bug had no useful information either, for
what it's worth.

But for the record, I disagree with Olli, precisely because of long and
hence unreadable bugs. If your commit message _can_ reasonably distill
a long bug discussion, then I think it should.

> If this is worth settling

For what it's worth, I don't think we need the "should the commit
message describe all the context?" discussion settled just to settle the
question of whether the commit message should _add_ anything to the
commit other than the bug number. Especially when the bug doesn't
explain the purpose of the commit either!

For the overall question here, I agree that we're not going to get
anywhere in a mailing list discussion. ;)

-Boris

[L. David Baron]

On Monday 2017-04-17 23:20 -0400, Boris Zbarsky wrote:
> On 4/17/17 10:45 PM, Jim Blandy wrote:
> > It seems like there is actually not a consensus on this. (I had thought
> > Smaug's view was the consensus, and found bz's post surprising.)
>
> Really? I know where Olli is coming from, but even in his view a commit
> message like the one I was talking about is not OK, I'm pretty sure.
>
> And note that in this case the bug had no useful information either, for
> what it's worth.

I think one of the things we agree on is that the commit message
should be worded as a description of the code change, not as a
description of the problem that's being fixed.

There doesn't seem to be as much agreement on how much background it
should provide, although I tend to be on the side of providing more
information in the commit message. I consider bugzilla to be
fundamentally the wrong UI for bug tracking as I described in
https://dbaron.org/log/20120816-bug-system and
https://dbaron.org/log/20130129-bugzilla and as a result I don't pay
much attention to the comments in bug reports; I mainly read the
summary.

-David

--
𝄞 L. David Baron http://dbaron.org/ 𝄂
𝄢 Mozilla https://www.mozilla.org/ 𝄂
Before I built a wall I'd ask to know
What I was walling in or walling out,
And to whom I was like to give offense.
- Robert Frost, Mending Wall (1914)

[Steve Fink]

Heh. Yes, bzexport will copy the commit message into a comment. 1 point
in its favor.

If you use it to create bugs (via the --new flag), it will use the first
line of the commit message as the default bug title. Which goes against
what has been repeatedly requested in this thread, in that the bug title
is what is wrong, the commit message is what is being done to fix it. 1
point against. (Though arguably this is ok for feature request bugs.)

(bzexport was also written in the mq era and has some deficiencies when
using it with a more modern workflow. It's fixable, but it needs some work.)

[Gregory Szorc]

On Mon, Apr 17, 2017 at 10:36 PM, Steve Fink <sf...@mozilla.com> wrote:

> On 04/17/2017 08:11 PM, Nicholas Nethercote wrote:
>
>> On Tue, Apr 18, 2017 at 11:34 AM, Ben Kelly <bke...@mozilla.com> wrote:
>>
>> I don't object to people writing longer commit messages, but that
>>> information needs to be in the bug. Our tools today (splinter and
>>> mozreview) don't do that automatically AFAIK. I think there is an hg
>>> extension you can get to do it with splinter.
>>>
>>> Yes, 'hg bzexport' defaults to copying the commit message into a bug
>> comment.
>>
>
> Heh. Yes, bzexport will copy the commit message into a comment. 1 point in
> its favor.
>

MozReview does exactly the same thing. But Bugzilla appears to hide
comments with the "mozreview-request" tag by default in the web UI. The
full commit message / attachment description comes across in the HTML
email, however. This seems like behavior that could be revisited if a bug
were filed with a compelling reason for the change.

>
> If you use it to create bugs (via the --new flag), it will use the first
> line of the commit message as the default bug title. Which goes against
> what has been repeatedly requested in this thread, in that the bug title is
> what is wrong, the commit message is what is being done to fix it. 1 point
> against. (Though arguably this is ok for feature request bugs.)
>

FWIW GitHub pull requests have the same problem. There is an inherent UX
design conflict between lowering the barrier to task completion [with
metadata that's "good enough"] versus asking the user to do too much in the
name of being ideal. In both cases, the tools are forcing the existence of
a "named grouping" (via a bug or pull request title/summary) onto
end-users. You can't argue too hard with that: summaries are convenient for
dashboards and tables. My personal opinion is the tools should optimize for
"throwing code over the wall" to get one step closer to landing and
minimize abandonment. So default heuristics for picking the summary feel
like a lesser evil than requiring the user to explicitly enter something.
And this is exactly what `hg bzexport --new` and GitHub's web UI for new
pull requests do by defaulting to the commit message for titles.

>
> (bzexport was also written in the mq era and has some deficiencies when
> using it with a more modern workflow. It's fixable, but it needs some work.)
>
>

[Anne van Kesteren]

On Mon, Apr 17, 2017 at 5:16 PM, Boris Zbarsky <bzba...@mit.edu> wrote:
> A quick reminder to patch authors and reviewers.

Is this documented somewhere so you can just point folks to
documentation if they get it wrong? E.g., for WHATWG standards there
is a README.md that (sometimes indirectly) points to
https://github.com/erlang/otp/wiki/Writing-good-commit-messages for
general commit message guidelines on top of which WHATWG has some of
their own conventions.


--
https://annevankesteren.nl/

[Mike Hommey]

Stories about how you end up with a particular change can be important
as to why the change was not done another way.

Mike

[Mike Hommey]

I disagree. Bugs have a lot of context, sure, but they also have lots of
irrelevant discussion when all you're looking for is context.

Summarizing the context in the commit message is, IMHO, the right thing
to do.

Mike

[Mike Hommey]

On Mon, Apr 17, 2017 at 09:34:39PM -0400, Ben Kelly wrote:
> On Mon, Apr 17, 2017 at 9:21 PM, Nicholas Nethercote <n.neth...@gmail.com
> > wrote:
>

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

> > With all due respect, I think you have a different view on this to at least
> > some people (and perhaps most people).
> >
>

> FWIW I agree with Olli. I look for a good one line summary of the change,
> but beyond that I find you really do need to look at the bug to get the
> full context.
>

> I don't object to people writing longer commit messages, but that
> information needs to be in the bug. Our tools today (splinter and
> mozreview) don't do that automatically AFAIK.

FWIW, I find this behavior of mozreview to be irritating. The commit
message are send in bugmail, but they are hidden in UI (although you can
unhide them). I can understand how multiple iterations of the patches
would leave multiple comments with essentially the same content, but the
opposite lack of anything displayed by default is also annoying.

> I think there is an hg extension you can get to do it with splinter.

Most things that attach patches automatically do that AFAIK. Obviously,
when you attach patches manually, it's up to you.

Mike

[Mike Hommey]

What sort of blame-browsing makes more than the first line of the commit
message a burden? I'm curious, because that doesn't match my use of
blame.

And I've done my share of archeology and for old enough stuff you often
end up with one of:
- a completely useless commit message *and* useless bug (if there's even
a bug number)
- a mostly useless commit message and some information in the bug.
You're lucky if it's all contained in a few sentences.
- a mostly useless commit message and tons of information in the bug,
that you have to spend quite some time parsing through.

In *all* cases, you have to go switch between your blame and bugzilla.
It's a PITA. Now, when you have everything in VCS, you just work with
your blame. Obviously, CVS-era commits are not going to change, but do
you really want to keep things in the same state for commits done now,
when you (or someone else) goes through blame in a few years?

Mike

[smaug]

On 04/18/2017 02:36 AM, Gregory Szorc wrote:

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.

[Ehsan Akhgari]

On 2017-04-18 12:30 AM, Mike Hommey wrote:
>> I've yet to see that to happen. What is crucial is fast way to browse
>> through the blame in time. So commit messages should be short and
>> descriptive. Telling what and why. (I very often need to go back to CVS era
>> code). I won't spend time reading several paragraphs of commit messages.
>> Those are just too long.
>> Basically first line should tell the essentials 'what' and maybe the most obvious 'why' and the next couple of lines explaining 'why' more precisely.
>> Don't write stories which will make blame-browsing slower.
>

> What sort of blame-browsing makes more than the first line of the commit
> message a burden? I'm curious, because that doesn't match my use of
> blame.

I can't say I have had this issue, and I also do a lot of code
archaeology as well. I suppose to a large extent this does depend on
what tools you use, perhaps. These days I use searchfox.org for a lot
of blame browsing that I do, which displays only the first line in the
default UI and only displays the full commit message when you look at
the full commit. I find this a pretty good balance.

> And I've done my share of archeology and for old enough stuff you often
> end up with one of:
> - a completely useless commit message *and* useless bug (if there's even
> a bug number)
> - a mostly useless commit message and some information in the bug.
> You're lucky if it's all contained in a few sentences.
> - a mostly useless commit message and tons of information in the bug,
> that you have to spend quite some time parsing through.
>
> In *all* cases, you have to go switch between your blame and bugzilla.
> It's a PITA. Now, when you have everything in VCS, you just work with
> your blame. Obviously, CVS-era commits are not going to change, but do
> you really want to keep things in the same state for commits done now,
> when you (or someone else) goes through blame in a few years?

Agreed.

Also even for newer code you often end up in the third category. Maybe
it's just me but I spend a lot of time reading old bugs, and I find it a
huge time sink to read through tons of comments just to find where the
actual discussion about why the fix was done in the way that it was done
happened in the bug. Sometimes I have to really read 100+ comments.
And the sad reality is that all too often I find very questionable
things in patches happen in bugs without any discussion whatsoever which
means that sometimes one can spend half an hour reading those 100+
comments very carefully to find out in the end that they have just
wasted their time and the bug actually did not contain the interesting
information in the first place.

I think I would probably sympathize more with the side of the argument
arguing for bugs as the source of the truth if this wasn't really true,
but I think part of the reason why people don't bother writing good
commit messages is that they assume that the reasons behind the
approaches they take and the design decisions and the like are obvious,
and while that may be true *now* and to them and the code reviewer, it
will not be true to everyone and in the future, and because of that if
you're not careful the important information is just not captured
anywhere. I hope we can all agree that it's important that we capture
this information for the maintainability of our code, even if we can't
agree where the information needs to be captured. :-)

[Boris Zbarsky]

On 4/18/17 3:00 AM, Anne van Kesteren wrote:
> On Mon, Apr 17, 2017 at 5:16 PM, Boris Zbarsky <bzba...@mit.edu> wrote:
>> A quick reminder to patch authors and reviewers.
>
> Is this documented somewhere so you can just point folks to
> documentation if they get it wrong?

Not yet. As this thread shows, there's some lively disagreement on what
such documentation should say... :(

-Boris

[Daniel Holbert]

The very basics (at least) are semi-documented here:

https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Committing_Rules_and_Responsibilities#Checkin_comment

I frequently point people at that ^^ when they get it wrong (and e.g.
use the bug title as the commit message).

~Daniel

On 4/18/17 12:00 AM, Anne van Kesteren wrote:
> On Mon, Apr 17, 2017 at 5:16 PM, Boris Zbarsky <bzba...@mit.edu> wrote:
>> A quick reminder to patch authors and reviewers.
>
> Is this documented somewhere so you can just point folks to

[smaug]

The important part of documentation should be in code comments, not commit messages.
We aren't too good commenting code.


Another thing, today I was looking at a bug which has several patches, and realized one can't really understand the big picture by looking at commit
messages of some particular commit. There needs to be some centralized place for that, and it is the bug. Going through each patches individually and
looking at the commit messages would be really painful way to see what and why was changed.

[smaug]

On 04/18/2017 04:24 PM, Ehsan Akhgari wrote:

[Alexander Surkov]

As a guy who stayed with Mozilla for a while, here's my two cents. I thumb up for Olli's descriptive short what and why commit messages in favor to long stories stretched over multiple patches. I'm also keen on going to the bug for details if a commit doesn't look quite clear with me.

I realize that bugs might be of thousands of comments, which makes them hard to read, but that's probably a case of bugs organization: one meta and several small dependent bugs might help here. I don't want to affirm that this approach suites every Mozilla module, but it seems be working well in relatively small modules like accessibility one.

[Boris Zbarsky]

On 4/25/17 10:50 AM, Alexander Surkov wrote:
> I don't want to affirm that this approach suites every Mozilla module, but it seems be working well in relatively small modules like accessibility one.

Just as a counterpoint... as non-regular contributor to the
accessibility module, I have a _very_ hard time making sense of
accessibility commits, precisely because the commit messages are not
often not very descriptive and the bugs are often hard to make sense of
for a non-expert.

I don't have this problem in cases where I'm similarly out of my depth
but commit messages contain more information.

-Boris

[Alexander Surkov]

I bet there's always room for improvements, and I hope this was a counterpoint for the example only, not for the bug organization approach.

Overall it feels with me that long comments vs check-the-bug is rather different styles, with their own benefits and disadvantages, and different people prefer one over another one.

[Boris Zbarsky]

On 4/25/17 1:07 PM, Alexander Surkov wrote:
> I bet there's always room for improvements, and I hope this was a counterpoint for the example only, not for the bug organization approach.

Sort of.

It was a counterpoint to "just check the bug; all the info is there".
Often it's not there, or not there in usable form.

If people included a summary of the discussion in the bug right about
when they commit, or had bugs that actually explained what's going on
clearly. I would be a lot more OK with the "check the bug" approach.

> Overall it feels with me that long comments vs check-the-bug is rather different styles

To be clear, I don't think commit messages need to be _long_. They need
to be _useful_. A commit message pointing to a particular bug comment
that explains all the ins and outs is no worse, from my point of view,
than a commit message that explains the ins and outs.

The problem I started this thread to address is things like a commit
message that says "flip this bit" and references a bug entitled "flip
this bit", with no explanation of what the bit does or why it should be
flipped. I hope we can all agree that _that_ is not OK. And it's far
too common.

-Boris

[smaug]

On 04/25/2017 08:20 PM, Boris Zbarsky wrote:
> On 4/25/17 1:07 PM, Alexander Surkov wrote:
>> I bet there's always room for improvements, and I hope this was a counterpoint for the example only, not for the bug organization approach.
>
> Sort of.
>
> It was a counterpoint to "just check the bug; all the info is there". Often it's not there, or not there in usable form.
>
> If people included a summary of the discussion in the bug right about when they commit, or had bugs that actually explained what's going on clearly. I
> would be a lot more OK with the "check the bug" approach.
>
>> Overall it feels with me that long comments vs check-the-bug is rather different styles
>
> To be clear, I don't think commit messages need to be _long_. They need to be _useful_.

Exactly. Recently I've seen some commit messages to be long but not useful, in the style "I did this and that", but not really explaining why.

> A commit message pointing to a particular bug comment that
> explains all the ins and outs is no worse, from my point of view, than a commit message that explains the ins and outs.

A major issue comes from bugs where the work is split to several patches. That is why bugs really need to explain it all too.

>
> The problem I started this thread to address is things like a commit message that says "flip this bit" and references a bug entitled "flip this bit",
> with no explanation of what the bit does or why it should be flipped. I hope we can all agree that _that_ is not OK. And it's far too common.
>

Yeah, definitely. The bug should be clear about what it is about to fix, and the commit message should also be clear what it is doing and why.
In some cases bug title is clear enough to explain why something is broken or should be changed, but not usually.
(I have perhaps a bit bad habit to file bugs with just descriptive title, especially in case of "Consider to do foo" bugs)


> -Boris
>

[Alexander Surkov]

Maybe we should have a style guide, explaining what makes a good commit message and what makes a good and descriptive bug, with number of (good and bad) examples.

[Boris Zbarsky]

On 4/25/17 4:27 PM, Alexander Surkov wrote:
> Maybe we should have a style guide, explaining what makes a good commit message and what makes a good and descriptive bug, with number of (good and bad) examples.

Yes, we should.

Maybe we should have a discussion at the all hands about this...

-Boris

[Selena Deckelmann]

Hi!

I'll take that as my cue to organize this.

I'll reach out off-list to the folks involved in this conversation and set
a time and an agenda.

Thanks!
-selena