Commit message policy?

Folks,

After looking at git / svn logs for a long time today, I realised that
the commit messages could be a bit more helpful for that purpose
(logs). The kernel has a good policy on commit messages, and I think
we could use some of that, at least as a strong guideline, not as a
hard rule.

The idea, AFAIK, is to have a title with 50 chars, separated by an
empty line and an infinite number of 80-col lines following. That
allows us to be very descriptive on the context, but succinct enough
on the title, so that logs can fit on a reasonably sized terminal +
git ids, etc.

Is that something we want to follow? I could only find the attribution
rule on the developer's policy (which we should definitely keep), but
would be good to have at least some policy on the commit message
itself.

Also, policies for messages when reverting patches, arc commits,
approval lists would be good to have documented, rather than keep
replying to people on the list.

cheers,
--renato

PS: I don't want us to revert patches with non-standard messages, only
encourage people to do it right.

Folks,

After looking at git / svn logs for a long time today, I realised that
the commit messages could be a bit more helpful for that purpose
(logs). The kernel has a good policy on commit messages, and I think
we could use some of that, at least as a strong guideline, not as a
hard rule.

The idea, AFAIK, is to have a title with 50 chars, separated by an
empty line and an infinite number of 80-col lines following.

I've never enforced a particular character limit on the first line, but I
try to make sure it's one line, then a blank line, then the rest - this
ensures that one line is the subject of the email sent to the mailing list.

A lot of people end up wrapping their summary line to 2 or 3 lines, which
results in an email subject (& as you mention, git/svn log summaries, etc)
that contain only a sentence fragment that might not be meaningful.

I think git log summaries don't mind really long initial lines - the tools
will wrap it, so far as I know.

I tend to follow something like that anyway so I think it's a good
idea, but if anything I'd prefer a longer title line than the body.
Particularly with the prefixes we tend to put in there, 50 chars is
hardly anything.

Cheers.

Tim.

I've never enforced a particular character limit on the first line, but I
try to make sure it's one line, then a blank line, then the rest - this
ensures that one line is the subject of the email sent to the mailing list.

Vim is really useful for that, as it colours the first 50 chars of the
title, so you know when you passed the limit.

I think git log summaries don't mind really long initial lines - the tools
will wrap it, so far as I know.

They do, but it messes up the formatting. I agree, that's not a really
bad thing...

cheers,
--renato

> The idea, AFAIK, is to have a title with 50 chars, separated by an
> empty line and an infinite number of 80-col lines following.

I tend to follow something like that anyway so I think it's a good
idea, but if anything I'd prefer a longer title line than the body.
Particularly with the prefixes we tend to put in there, 50 chars is
hardly anything.

Yep, that's my main concern about imposing a (quite short) limit on the
summary line.

I only mentioned 50-col because that's what Vim helps me do automatically. :slight_smile:

I'm fine with larger titles, but not larger than 70-col, which should
be plenty, since we don't have complex branching in git. GMail can
cope with up to 90 chars (+preamble) in the subject.

cheers,
--renato

The idea, AFAIK, is to have a title with 50 chars, separated by an
empty line and an infinite number of 80-col lines following.

I tend to follow something like that anyway so I think it's a good
idea, but if anything I'd prefer a longer title line than the body.
Particularly with the prefixes we tend to put in there, 50 chars is
hardly anything.

I tend to look at this like I look at superExtremelyLongFunctionNames:
if it needs a really long description for the summary, maybe it's better
to break it into a few smaller patches. My $0.02, anyway... :slight_smile:

Cheers,

Jon

I try to wrap summaries at 80 columns by hand (so as to make reading logs in a shell easier) but even 70 columns is short with "[analyzer] " at the front. I’m okay with picking a length as a target; I just don’t want us to start sending complaints to people whose first lines go over the limit.

(Now, people whose summary “line” is broken up into multiple lines, sure, because that affects e-mail and git (and SVN these days? not sure).

Jordan

Absolutely! Should be just a guideline.

cheers,
--renato

Just as an example, the case I've run into is something like:

[analyzer] Separate RegionStore::getElementRegion and ...getFieldRegion

which, even with the abbreviation, is 72 characters long. I can fit it into 70 with an ampersand, but...

Jordan

+1 for a having a short leading sentence followed by a blank line. This is always the email subject, so plan accordingly.

IMO the rest doesn’t matter and isn’t worth codifying.

IMO, none of this is worth codifying really...

We could certainly have a little guide for how to write awesome commit logs
and point people at it. But this is one of (many) aspects of getting ramped
up on the practices and patterns of the community. Not sure it ever really
makes sense to try to codify so much as explain it...

I agree we shouldn't codify or enforce, but educate and remind people
of good practices.

How about the section attached?

cheers,
--renato

commit-msg-policy.patch (1.95 KB)

That seems mostly reasonable. I’d try to make it more concise, though. The coding standards and developer policy docs should be short.

+Commit message

Right, version 2 attached. Does everyone agree with it?

cheers,
--renato

commit-msg-policy.patch (1.4 KB)

From: "Renato Golin" <renato.golin@linaro.org>
To: "Reid Kleckner" <rnk@google.com>
Cc: "Clang Dev" <cfe-dev@cs.uiuc.edu>, "LLVM Dev" <llvmdev@cs.uiuc.edu>
Sent: Thursday, September 25, 2014 11:14:37 AM
Subject: Re: [LLVMdev] [cfe-dev] Commit message policy?

> That seems mostly reasonable. I'd try to make it more concise,
> though. The
> coding standards and developer policy docs should be short.

Right, version 2 attached. Does everyone agree with it?

I think this looks good. I'd say, "because that fits better" instead of "since that'd fit better".

Also, I'd like to add a note that all commit references should be to svn revision numbers, and not git hashes from a git mirror. In addition, all reverts must provide a rational.

Thanks,
Hal

I don't specifically disagree with it, but I don't think all of it is
useful. Let me be more specific though:

+Commit message

Dear All,

Two quick comments:

1) Don't suggest that titles be 70 characters but that 80 is okay. Just say they should be at 70 characters and leave it at that.

2) +1 for Chandler's comment that what is really needed are ideas on how to write meaningful descriptions of what was changed. Poorly written descriptions of what was changed is the number one reason I curse at commit logs.

-- John T.

Hi John, Chandler, I completely agree.

Cheers,
Renato

Hi,

I haven't got any comments to add that haven't already been discussed but I thought it would be worth mentioning that Phabricator's documentation has a fairly good section on good commit messages at https://secure.phabricator.com/book/phabflavor/article/writing_reviewable_code/#write-sensible-commit-me.