Bikeshedding commit message policy - Round 3 - Fight!

Folks,

On review http://reviews.llvm.org/D8197, we're basically down to two
bikeshedding issues:

1. Title tags

Some people use "[CSE] Change blah", others use "CSE: Change blah". I
hadn't put anything regarding tags because not everyone use it and
when they do, it's slightly different. I personally don't think it's a
reason to argue about, so I'm in favour of removing the comment
altogether and let people do what they feel is best for their own
commits.

Anyone feel strongly about this? I vote for removing the paragraph and
let people realise they can do that by looking at the past commit
messages.

2. Attribution

The dev policy (http://llvm.org/docs/DeveloperPolicy.html#attribution-of-changes)
gives an example of how to do it: “patch contributed by J. Random
Hacker!”. I personally took that as a guideline, not a rule, and never
really used the exclamation mark, nor I say "contributed", and that
seems what most people do, too. Since having them differently on both
places will bring people to bike shed, I think we need to stick to
one, or be explicitly vague about it.

People have scripts that rely on that, and since it seems to be
working, I'm pretty sure it *doesn't* rely on the word "contributed"
not it relies on the exclamation mark. I'm strongly in favour of not
requiring any of it.

Anyone feels that strongly about "contributed" or the exclamation mark?

I see three solutions here:

1. We stand by that exact phrase, since it's "kosher". I'm against it.

2. We just say that a line that contains the words "patch", "by",
"<name><punctuation>" will be parsed by attribution. I'm against it,
as this will start another bike shed on the exact regular expression
to use. Not to mention this will be an internationalisation and
abbreviation hell.

3. We say "Patch by Foo Bar." and let people be reasonably creative. I
vote for this one.

cheers,
--renato

From: "Renato Golin" <renato.golin@linaro.org>
To: "LLVM Dev" <llvmdev@cs.uiuc.edu>, "Clang Dev" <cfe-dev@cs.uiuc.edu>
Sent: Saturday, March 14, 2015 2:57:20 PM
Subject: [cfe-dev] Bikeshedding commit message policy - Round 3 - Fight!

Folks,

On review http://reviews.llvm.org/D8197, we're basically down to two
bikeshedding issues:

1. Title tags

Some people use "[CSE] Change blah", others use "CSE: Change blah". I
hadn't put anything regarding tags because not everyone use it and
when they do, it's slightly different. I personally don't think it's
a
reason to argue about, so I'm in favour of removing the comment
altogether and let people do what they feel is best for their own
commits.

Anyone feel strongly about this? I vote for removing the paragraph
and
let people realise they can do that by looking at the past commit
messages.

I used to use CSE:, but have now switched to using [CSE] because that seems to be the prevailing convention (and is somewhat more visually distinctive). I think it makes sense to codify that convention, but not to require them. Sometimes, there is nothing appropriate to use. Sometimes, the first or second word of the commit message is naturally the same as what the title tag would be, and so including the title tag seems redundant.

2. Attribution

The dev policy
(http://llvm.org/docs/DeveloperPolicy.html#attribution-of-changes)
gives an example of how to do it: “patch contributed by J. Random
Hacker!”. I personally took that as a guideline, not a rule, and
never
really used the exclamation mark, nor I say "contributed", and that
seems what most people do, too. Since having them differently on both
places will bring people to bike shed, I think we need to stick to
one, or be explicitly vague about it.

People have scripts that rely on that, and since it seems to be
working, I'm pretty sure it *doesn't* rely on the word "contributed"
not it relies on the exclamation mark. I'm strongly in favour of not
requiring any of it.

Anyone feels that strongly about "contributed" or the exclamation
mark?

I see three solutions here:

1. We stand by that exact phrase, since it's "kosher". I'm against
it.

2. We just say that a line that contains the words "patch", "by",
"<name><punctuation>" will be parsed by attribution. I'm against it,
as this will start another bike shed on the exact regular expression
to use. Not to mention this will be an internationalisation and
abbreviation hell.

3. We say "Patch by Foo Bar." and let people be reasonably creative.
I
vote for this one.

I agree. I think the important part is that the name appear in an obvious context. That way, if I search my Inbox for the name, the relevant commit will appear, and it will be obvious why without reading a lot of text.

-Hal

I agree with you that [CSE] is the most visually striking, but I
wonder how much do we want to code when to use them. There will always
be arguments to all sides, and I think this is not a topic important
enough for us to make it official.

See how the exclamation mark became self important on the attribution
and you'll see what I mean. I think common sense will always prevail
if we don't try to push too many standards.

cheers,
--renato

From: "Renato Golin" <renato.golin@linaro.org>
To: "Hal Finkel" <hfinkel@anl.gov>
Cc: "LLVM Dev" <llvmdev@cs.uiuc.edu>, "Clang Dev" <cfe-dev@cs.uiuc.edu>
Sent: Sunday, March 15, 2015 10:48:37 AM
Subject: Re: [cfe-dev] Bikeshedding commit message policy - Round 3 - Fight!

> I used to use CSE:, but have now switched to using [CSE] because
> that seems to be the prevailing convention (and is somewhat more
> visually distinctive). I think it makes sense to codify that
> convention, but not to require them. Sometimes, there is nothing
> appropriate to use. Sometimes, the first or second word of the
> commit message is naturally the same as what the title tag would
> be, and so including the title tag seems redundant.

I agree with you that [CSE] is the most visually striking, but I
wonder how much do we want to code when to use them.

I don't want to code when to use them. But it makes sense to say, "If you want to include a title tag, do it like this...".

There will
always
be arguments to all sides, and I think this is not a topic important
enough for us to make it official.

See how the exclamation mark became self important on the attribution
and you'll see what I mean. I think common sense will always prevail
if we don't try to push too many standards.

I think this is different for a number of reasons.

-Hal

I'm ok with that.

So, do we have consensus?

1. Don't require, but recommend using for tags.

2. Don't specify attribution more than just "patch by Foo." and expect
people to be sensible. Current scripts already work with a good
variation anyway.

cheers,
--renato

Can you post the entire revised diff that you want to include? Is it Diff 21913 on Phab? If so, LGTM.

-Chris

Hi Chris,

Here's the final version:

http://reviews.llvm.org/D8197

cheers,
--renato

Looks fine to me, here are some suggested wording changes to make it flow better. I also added a request to include bugzilla number.

-Chris

Commit messages

Thanks Chris, committed on r232334.

I've included all your changes, thanks for the help.

cheers,
--renato