Talk:Gerrit/Commit message guidelines/Archive 1

From mediawiki.org
Latest comment: 7 years ago by Fomafix in topic Wrapping

Wrapping

Is Gerrit really unable to display text correctly if not manually wrapped, as the text currently suggests? Seems worth a bug upstream. --Nemo 21:09, 30 March 2012 (UTC)Reply

That is by design I guess. Some old people, like me, tends to like having the text already wrapped :-D Antoine "hashar" Musso (talk) 18:54, 31 March 2012 (UTC)Reply

The new Gerrit version wraps the commit message after 80 characters. The current suggestion to wrap the commit message body after 100 characters leads to text looks like a comb. Example: https://gerrit.wikimedia.org/r/307133 The guidelines should get updated. --Fomafix (talk) 10:14, 30 December 2016 (UTC)Reply

Bug number in parentheses

Is it set in stone that the bug number must be in parentheses?

This wastes two character in a line that is very limited. Parentheses are not needed to make the bug number stand out. Gerrit shows it as a link and makes it distinct enough.

I suggest removing the parentheses. --Amir E. Aharoni (talk) 06:39, 18 June 2012 (UTC)Reply

I would prefer to move from 65 to 72 chars for the first line, because these () adds in readability.
Or maybe should we more generic in the first line instead to want to give all information there. --Dereckson (talk) 07:03, 18 June 2012 (UTC)Reply

Patch set documentation

Since the first commits to the Wikimedia Gerrit repo, it has been the practice of describing subsequent patch sets (amends) in the commit message. Dereckson commented in this commit that this is not a good idea, and I tend to agree.

Descriptions of amends are interesting in the course of the patch development. They are less relevant after merging, because the final repo only shows the last amend. So it may be better to suggest to write the amend descriptions in the review comments in Gerrit and to write the commit message as a description of the latest amend.

Other thoughts? --Amir E. Aharoni (talk) 06:39, 18 June 2012 (UTC)Reply

I just send the new patch set then comment on it. This way people receive a mail notification describing what changed in the new patchset. Once the commit is merged, we probably have no interest in how we got there. Antoine "hashar" Musso (talk) 20:04, 27 June 2012 (UTC)Reply

(bug 12345) or (Bug 12345)?

There are currently tho school in commits.

About two third of the commits use "(bug ...)" but some developers prefer to capitalize, as it's the first word.

Shouldn't we take a harmonization decision between those two variants?

Why it's important to have an harmonization on this matter? That will improve readability when you read the list of the commits to distinguish between the bug fixing changesets and the others. As long as there are (Bug ....) and (bug ...), we don't have this visual help to see there are lines starting by the same content. --Dereckson (talk) 08:33, 19 June 2012 (UTC)Reply

Suggesting to describe before/after

Usually people only describe what they changed in the commit; that is, how the software is supposed to behave after the change. Very frequently the commit message doesn't describe the situation before the change. It would be useful to describe the situation before the change and the reason that it should be changed, to help the reviewer who may misunderstand the context without it. This may not be required for commits that fix bugs that are documented in Bugzilla, but many commits are out-of-the-blue patches without a Bugzilla context.

An example of a common current commit message:

Change the width of the date field to 16.

An example of the proposed commit message:

Before this change the width of the start date field was 12, which<br />
was too narrow for the long date format used there.
    
I changed it to 16, so that the whole date would be seen.

(It's based on an actual commit: https://gerrit.wikimedia.org/r/#/c/14780/ .)

What do you think about adding this to the best practices? --Amir E. Aharoni (talk) 21:03, 9 July 2012 (UTC)Reply

Bug number in commit messages

Reminder to watchers: there isn't a very robust consensus about the removal of bug number from first line of commit message. Remember that there isn't any tool to list the bugs that were fixed in a release, so manually updating release notes in the same commit that fixes a bug is now your only option to let people know. --Nemo 14:30, 10 March 2013 (UTC)Reply

This is not correct. Reedy and Chad updated the MW release details script so that the changelog page (e.g. that for 1.21wmf11) now lists the bug number after the title, regardless of which format you use. However, if you use the new format you allow gerrit to index the bug changes too, which is very helpful. Please, everyone, do follow the new guidelines. Jdforrester (WMF) (talk) 04:38, 11 March 2013 (UTC)Reply
Thanks for noting this. However, that's only the WMF's versions' notes, I was talking of the RELEASE-NOTES file. Are there plans to automate that too? Please don't encourage people to disregard coding conventions on release notes. --Nemo 06:00, 11 March 2013 (UTC)Reply
… but this page is about the commit messages, not the manual release notes, which are (as you stated) mentioned there. Jdforrester (WMF) (talk) 16:43, 11 March 2013 (UTC)Reply
In fact I just wanted to highlight that the change to the guideline didn't point to a consensus (which I've no idea if exists or not now) and that both guidelines have to be followed (or none), but see talk for that one. --Nemo 16:52, 11 March 2013 (UTC)Reply

Notify Bugzilla

The doc says to put "Bug: bugnumber" in the footer (before the Change-Id) - and that this is supposed to notify Bugzilla of any changes.

This does not seem to work, see:

https://gerrit.wikimedia.org/r/#/c/150568/

The bug gets a link OK, but no notification is sent. Seems like "bug bugnumber" in the header does work. Doc. error or config error? --Bep (talk) 23:30, 30 July 2014 (UTC)Reply

@Bep: As the footnote says:
As with all headers/footers: Camel case the name, then a colon (":"), then one space. Similar to the Git commit, HTTP and E-mail headers, blank lines here would cut off the footer and wrongly include the former part in the body.
The blank line you have between the bug link and the change ID means it doesn't work.
Jdforrester (WMF) (talk) 23:33, 30 July 2014 (UTC)Reply
Got it! Thanks! --Bep (talk) 23:42, 30 July 2014 (UTC)Reply