please use meaningful commit messages

15 views
Skip to first unread message

Evan Martin

unread,
Apr 14, 2010, 8:03:04 PM4/14/10
to chromium-dev
I've been seeing commit messages go by in this project of the form
"fix a bug" or "add files x, y, z and modify files q, r" or "fix the
foobar test". These are not helpful.

We need useful commit messages for a few important reasons, but
they're all related:
1) When trying to track down a regression, it's common to look at the
changes in the regression window and see which commits likely caused
it. Without a meaningful message it's harder to tell whether a given
change is relevant.
2) When making a new release, the release manager scans the commit log
to see what has changed. When you say "fixed a bug" they'll have to
go through and click on the bug link to see what the bug actually was.
3) Etc. etc. Here's a nice longer essay on the subject:
http://blog.spearce.org/2010/02/why-commit-messages-matter.html

In general, our culture is to include relevant information about what
the code *does* as comments in the code.
So your commit message should answer the other important questions:
what this particular change changes, and *why* you are making this
change.
(If you want to be extra-helpful, making the first line of your commit
message as short and descriptive as an email subject makes some of our
tools.)

As a reviewer, your responsibility is to review both the code and the
commit message, which is the text shown at the top of the review.
Please help keep our standards high.

Lei Zhang

unread,
Apr 14, 2010, 8:12:44 PM4/14/10
to ev...@chromium.org, chromium-dev
Also, "committing [codereview url] for [non-committer]" is annoying.
Please include the original CL description.

On the plus side, I like commit messages with a prefix. It makes it
easy to figure out the scope / importance of the commit. i.e. Mac: Add
shiny iThing feature; Cleanup: Remove dead code in foo::bar().

> --
> Chromium Developers mailing list: chromi...@chromium.org
> View archives, change email options, or unsubscribe:
>    http://groups.google.com/a/chromium.org/group/chromium-dev
>

Peter Kasting

unread,
Apr 14, 2010, 8:23:21 PM4/14/10
to the...@chromium.org, ev...@chromium.org, chromium-dev
On Wed, Apr 14, 2010 at 5:12 PM, Lei Zhang <the...@chromium.org> wrote:
Also, "committing [codereview url] for [non-committer]" is annoying.
Please include the original CL description.

My template for this:

<Change description>.  Original patch by <name> (see <original review URL>), r=<reviewer>.

This has all the bits you could want in a fairly readable order.

PK

Paweł Hajdan, Jr.

unread,
Apr 15, 2010, 2:48:05 AM4/15/10
to ev...@chromium.org, chromium-dev
Also, I think it would be useful to state the reason for a revert, at least in the TBR-ed review message that goes to the original author. It is important to know why a change was reverted - was it a test failure (which test? are there some logs?), compile failure, perf regression, etc.

Reply all
Reply to author
Forward
0 new messages