LWN: Comments on "What's missing from our changelogs"

What's missing from our changelogs

mp — Sat, 03 Aug 2013 22:54:11 +0000

I guess you mean mid:.

What's missing from our changelogs

mathstuf — Tue, 30 Jul 2013 16:07:20 +0000

> we should probably dictate that maintainers *must*not*do*that* (--no-ff please!)

Does setting the noff option in gitconfig still force pull-merges without pull.rebase to true? Merges with --no-ff are great, but pull merges are really quite a bit worse, IMO. (I don't care how often you pull from a remote, just don't modify the tree when you do it). Also, it means that if there are conflicts, they happen when merging a remote into your branch which is backwards since your branch is topologically older.

What's missing from our changelogs: Reverts

jezuch — Tue, 30 Jul 2013 08:25:24 +0000

> A revert is not the same thing as making the commit never exist. Often they do not apply cleanly and it's very hard for a reviewer to know whether the author of the revert had to apply logic to resolve conflicts.

It would be useful if revert commits looked like (or simply were) merges. I sometimes do it by hand, but that's awkward.

What's missing from our changelogs

neilbrown — Tue, 30 Jul 2013 04:38:50 +0000

I'd suggest that was a poor example as it was merged directly by Linus, so there is only one interesting merge.

fast-forward merges are certain to cause problems, and we should probably dictate that maintainers *must*not*do*that* (--no-ff please!)(they probably already do?). Assuming a lack for fast-forward patches, "git name-rev" can help.

e.g.

$ git name-rev 453807f3006757a5661c4000262d7d9284b5214c
453807f3006757a5661c4000262d7d9284b5214c tags/v3.10-rc1~14^2~20^2~3^2~29

Then apply the following (which can obviously be scripted):

% git log -n1 --format=short tags/v3.10-rc1~14^2~20^2~3^2~29
commit 453807f3006757a5661c4000262d7d9284b5214c
Author: Lars-Peter Clausen <lars@metafoo.de>

    ASoC: ep93xx: Use ep93xx_dma_params instead of ep93xx_pcm_dma_params
% git log -n1 --format=short tags/v3.10-rc1~14^2~20^2~3     
commit 9eb8ae727dcb9f2530a895ee6b3496592853709d
Merge: 5561f17 6f1fd93
Author: Mark Brown <broonie@opensource.wolfsonmicro.com>

    Merge remote-tracking branch 'asoc/topic/dma' into asoc-next
% git log -n1 --format=short tags/v3.10-rc1~14^2~20    
commit 2fc565e4eaf8fc633bfc741b90e1f28dba732ee1
Merge: 7fc7d04 5cc50fc8
Author: Takashi Iwai <tiwai@suse.de>

    Merge tag 'asoc-v3.10-3' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound into for-next
% git log -n1 --format=short tags/v3.10-rc1~14     
commit 05a88a43604abb816dfbff075bb114224641793b
Merge: daf799c 6c35ae3
Author: Linus Torvalds <torvalds@linux-foundation.org>

    Merge tag 'sound-3.10' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound

and you have the merge commits that merged the patch into the trunk and the people responsible. (interesting commits to experiment with can be found via

git log | git name-rev --stdin | grep '~.*~.*~.*~.*~'

with varying numbers of '~' in the 'grep')

What's missing from our changelogs

akpm — Mon, 29 Jul 2013 23:31:24 +0000

That O_TMPFILE patch is a travesty. wtf?

What's missing from our changelogs: Reverts

PaulMcKenney — Mon, 29 Jul 2013 21:55:19 +0000

Good to know! Lucky for me that I have never sent a pull request for a reverted commit, I would say. ;-)

What's missing from our changelogs: Reverts

aliguori — Mon, 29 Jul 2013 18:32:55 +0000

Yes, reverts need a Signed-off-by. git-revert even has a -s option for it.

A revert is not the same thing as making the commit never exist. Often they do not apply cleanly and it's very hard for a reviewer to know whether the author of the revert had to apply logic to resolve conflicts.

What's missing from our changelogs

marcH — Sat, 27 Jul 2013 22:59:40 +0000

Yes I was assuming something like Message-ID://

What's missing from our changelogs

mathstuf — Sat, 27 Jul 2013 18:41:01 +0000

Is there a way to tell a Message-Id from an email address? How would you determine that some string is an ID without something like message-id: (like email:)?

What's missing from our changelogs

corbet — Sat, 27 Jul 2013 13:36:02 +0000

Again, Linus seems to think differently; he follows the same rules as everybody else for anything but version number tweaks. I think this makes sense; there's lots of kernel repositories, only one controlled by Linus. Adding an implicit "no signoff means signed-off-by: Linus" rule for all of them strikes me as inconsistent and weird.

As for whether he signs off on a version number change, it doesn't matter that much. I was just describing the data for the curious.

What's missing from our changelogs

corbet — Sat, 27 Jul 2013 13:31:17 +0000

Interesting, I just tried that with randomly chosen commit b4419e1a15905191661ffe75ba2f9e649f5d565e (a GPIO fix) and found that it listed the thermal and tracing trees, which are unlikely to have hosted that patch. --ancestry-path prints all intervening commits, so there's a lot of extra noise, while, of course, any fast-forward merges will be missing. A useful tool but not a definitive answer.

What's missing from our changelogs

rjw@sisk.pl — Sat, 27 Jul 2013 02:18:18 +0000

Please allow me to disagree with this:

Without a chain of signoff lines, we lose more than a picture of which maintainers might have reviewed the patches; we also lose track of the path by which a patch finds its way into the mainline.

Given a commit hash you can always do

$ git log --ancestry-path --merges <commit>..HEAD

go to the bottom of the log and see whose hands the commit has gone through.

What's missing from our changelogs

aegl — Fri, 26 Jul 2013 21:35:19 +0000

The world should be able to see "Author: Linus Torvalds" ... and not need anything else for patches that Linus applies himself (Things are a bit different when Linus writes a patch and then asks a sub-maintainer to apply it ... then there is some value in the Signed-off-by chain).

You called out specifically changes to version that add other data (pointing to the Linux for Workgroups logo). Would this really have been better if it had a "Signed-off-by:" line?

What's missing from our changelogs

corbet — Fri, 26 Jul 2013 20:23:28 +0000

I'm confused by this comment. Linus does sign off patches that he writes, or that he applies directly (not via a pull). And that makes sense; the purpose of a signoff isn't just for Linus to know, it's for the world to know who took responsibility for the patch. What he doesn't assign is version number updates.

What am I missing here?

What's missing from our changelogs: Reverts

PaulMcKenney — Fri, 26 Jul 2013 18:25:12 +0000

Eight of these are reverts of earlier commits. Do reverts need Signed-off-by?

What's missing from our changelogs

aegl — Fri, 26 Jul 2013 17:10:18 +0000

You mention Linus not adding sign-offs to patches that he applies himself.

What would be the purpose of such a thing. So that Linus would know that he had claimed the legal authority to post the patch that he just wrote? It all seems a bit self-referential and involves too much of Linus talking to himself.

What's missing from our changelogs

bfields — Fri, 26 Jul 2013 11:58:18 +0000

Or to put it another way: if someone takes a complicated change, carefully splits it into small independent steps, and leaves the explanation on the first commit--then they get dinged for writing inadequate commit messages?

Often the reason Al's patches lack changelog bodies is because he does the hard work of figuring out how to get from point A to point B in trivial self-explanatory steps--something he deserves credit for. (What *does* get lost, I think, is motivation: for example, motivation for the readdir api change ("dealing with ->f_pos races in ->readdir() instances for good") is only in the cover letter, as far as I can tell, not in any changelog or code comment (but I may have missed it, or maybe it's there in changes to problematic filesystems, I didn't check).)

In any case, I don't think we can draw useful conclusions from simple summaries of the commit statistics--careful examination of representative cases would seem more instructive.

What's missing from our changelogs

nevets — Fri, 26 Jul 2013 03:02:52 +0000

I don't see the problem with the way we do the links. The link includes the message id. You get the best of both worlds.

What's missing from our changelogs

aliguori — Fri, 26 Jul 2013 00:53:51 +0000

Empty commits dont survive a rebase which makes it hard to use for topic branches.

What's missing from our changelogs

broonie — Thu, 25 Jul 2013 19:51:59 +0000

Yeah, this is particularly true for a lot of the patches which Axel Lin submits - I noticed him appearing in those lists but actually his changelogs are generally very high quality, detailed when they have to be but not tediously long when they're obvious.

What's missing from our changelogs

marcH — Thu, 25 Jul 2013 19:02:31 +0000

Or even better: desktop systems could somehow add message IDs to the list of MIME types and handlers they support. Then you could just click on them and this would automatically open the thread in your mail indexer of choice.

What's missing from our changelogs

jiiksteri — Thu, 25 Jul 2013 17:46:24 +0000

git notes are made for this, it's a shame they didn't standardize on adding signoffs and other tags as notes once the feature matured.

As git notes are stored in a separate ref in the repo (which is what makes them not alter history in the first place), you also have to tag and sign the note ref separately if you want to trust any notes information.

No idea if faked signoff attempts are that big a problem though :)

What's missing from our changelogs

aliguori — Thu, 25 Jul 2013 16:58:40 +0000

Fortunately Linus doesn't control QEMU development :-)

A message-id is infinitely more useful than a link. There is no programmatic way of getting a message-id from a link so you can't generate email responses without it.

People just need to learn how to use mid.gmane.org :-)

What's missing from our changelogs

nevets — Thu, 25 Jul 2013 13:02:56 +0000

We use to use LKML-Reference: tags that just included the message-id, but Linus hated it:

https://lkml.org/lkml/2011/3/28/460

Which was changed to the link format of:

Link: http://lkml.kernel.org/r/<message-id>

That way we have a link to the message that you can easily find, and if you are worried about that link disappearing, you have the message-id in the link. The best of both worlds.

But you should know that Linus hates just having the message-id as a tag.

What's missing from our changelogs

hmh — Thu, 25 Jul 2013 12:39:50 +0000

You could have the entire patch series as a temporary (topic) branch, and the cover letter as the [empty] merge commit log message, and you can do that right now.

This works well only when the patch stack is going in through the same tree, obviously.

As for notes, their out-of-band nature will complicate a secure workflow somewhat, but it should be possible to do it.

What's missing from our changelogs

aliguori — Thu, 25 Jul 2013 12:29:11 +0000

I do the same thing but I do it in the form of a Message-id: tag. That way you don't have to worry about links breaking over time.

Here's an example:

http://git.qemu.org/?p=qemu.git;a=commit;h=fd1d9926e91f42...

Besides being a good way to associate a commit with a mail thread, it is useful for generating automatic "Thank you" notes when a patch is applied.

What's missing from our changelogs

anmoch — Thu, 25 Jul 2013 09:40:01 +0000

You can have, and show, multiple notes refs (collections of notes) at the same time. You can also merge changes to notes refs. What's really missing right now is a "good", predefined sharing scheme(*), and a good use-case.

It seems like the kernel collecting further signoffs and similar tags could provide the latter. I can try to get the ball rolling, but it would help massively if a group of interested kernel devs would volunteer as guinea pigs :-)

(*) you can share them by manually configuring push/fetch specs.

What's missing from our changelogs

lrothc — Thu, 25 Jul 2013 09:29:02 +0000

Ah, this looks good. I've been using the "find message ID" kind of thing when referencing email discussions, but it never occurred to me to include it in the tags section of the commit log.

But I think it would be better if this became more standard. Maybe if git itself would save the message ID so it could be easily looked up.

What we're missing here is a way to make it possible to find email discussions by looking at *any* commit in the git log. If there is a commit for which the message ID cannot be found in any public mailing lists, that would be a red flag telling us that the commit was not reviewed publicly.

What's missing from our changelogs

lacos — Thu, 25 Jul 2013 08:44:48 +0000

Interesting! Thanks!

(Still I like to end up immediately in the threaded / frames interface.)

What's missing from our changelogs

johill — Thu, 25 Jul 2013 08:32:03 +0000

There's an easier redirect one for gmane:

http://mid.gmane.org/20130718184712.GA4786@redhat.com

What's missing from our changelogs

lacos — Thu, 25 Jul 2013 08:02:15 +0000

This. Message-IDs are very important, and it's good that some public archives support search by msgid. For example, the URL format for gmane is

http://news.gmane.org/find-root.php?message_id=2013071818...

A further improvement is storing the original subject (including the "bag of tags" part) of the patch email (if the patch was posted to and applied from a list) as a "pseudo header". This helps identifying a series using just the git commit log (due to the patch numbering being captured).

What I miss is some way to save cover letters as standalone, no-code-change commits.

What's missing from our changelogs

error27 — Thu, 25 Jul 2013 07:38:07 +0000

"One might contend that patches become less subject to review once they enter the Git stream."

Hopefully everything went to a list as well. I don't think anything gets reviewed again when it hits git. I know I review everything on the staging mailing list, but I never review git patches.

Maintainers often don't apply patches when the merge window is open so there is sometimes a delay before the patch gets reviewed and applied. But for bystanders, if no one comments on the patch within 3 days then there probably won't be any comments. In other words, if I don't comment on the patch after three days, that means I have Ack-ed it.

What's missing from our changelogs

aliguori — Thu, 25 Jul 2013 03:52:59 +0000

patchwork is nice but doesn't handle threads and cannot automatically handle commit status and resubmissions. I have more scripts to handle that too.

What's missing from our changelogs

stressinduktion — Thu, 25 Jul 2013 03:07:05 +0000

Have a look at patchwork[0]. It is used by e.g. the netdev[1] folks. It handles commit-tags, makes it easy for contributors to follow their patches or invites one to just browse the queue to do review.

[0] http://jk.ozlabs.org/projects/patchwork/
[1] http://patchwork.ozlabs.org/project/netdev/list/

What's missing from our changelogs

nevets — Thu, 25 Jul 2013 02:01:33 +0000

The scripts I use extract the message id from the email from a patch that is sent to me and its automatically added to the change log. They look like this:

Link: http://lkml.kernel.org/r/20130718184712.GA4786@redhat.com

That maps to marc.info/?i=20130718184712.GA4786@redhat.com which is the email of the patch. Usually from that, you can get the thread, which *is* very useful. I just used it a few minutes ago to figure out why some function was called in the MIPS code.

What's missing from our changelogs

apoelstra — Thu, 25 Jul 2013 01:00:45 +0000

> I'm confused by the comment on readability; the point of the explanation area is to make the patch easier to understand, after all. I didn't suggest populating it from your spam folder. And I'll confess to being totally thrown by the mention of bisectability - how on earth is that affected by changelog contents?

I think the implication was that people ought to keep piling things into a single commit until that commit is complicated enough to justify a full explanation.

Having said that, I read the article and understood it the way you intended. (And I think most developers would have no problem recalling examples of commits where one line is sufficient, even if you had not pointed it out.)

What's missing from our changelogs

josh — Thu, 25 Jul 2013 00:48:50 +0000

I'd love to see some analysis of Reviewed-by, as well.

What's missing from our changelogs

marcH — Wed, 24 Jul 2013 23:29:01 +0000

I think git is missing some metadata or "properties" concept. Another, somewhat related example where this would be useful:

https://mail.gna.org/public/stgit-users/2013-07/msg00011....

Granted, the security (as implemented by the SHA1 chains) does not make that easy.

What's missing from our changelogs

aliguori — Wed, 24 Jul 2013 23:02:05 +0000

The chain of Signed-off-by is there just not in the expected way provided that maintainers 1) set no-ff and 2) add a Signed-off-by to the merge commit.

If you do that (and we do in QEMU) then you can walk the full history.

The practical problem with Reviewed-by/Tested-by is it's a pain in the ass to add them as a maintainer. I have some scripts to do this given an mbox and have been planning to generalize but haven't gotten around to it. I suspect a few other people have similar scripts floating around.

The script I use is available at http://git.codemonkey.ws/cgit/mbox-filter.git

What's missing from our changelogs

blackwood — Wed, 24 Jul 2013 22:43:49 +0000

In drm/i915 I simply demand that review needs to be reflected in the commit, no matter what. So either the patch gets updated or the commit message needs improvement since if it would have been perfect no questions would have popped up in review.

For similar reasons I also insist that the revision log of individual patches is kept as part of the commit message (and not hidden below the -- line). We also use a bit more elaborate commit message citation layout (essentially git show up to the patch headline) so that author, committer, sha1 and headline is all there at a glance.

It's a lot more work than onleliners but imo enforcing high standars for commit messages is really worth it when digging through history.