Re: [PATCH v2 5/9] revert: libify pick

[Christian Couder <chriscool@tuxfamily.org>]

On Friday 21 August 2009, Junio C Hamano wrote:
> Christian Couder <chriscool@tuxfamily.org> writes:
> > From: Stephan Beyer <s-beyer@gmx.net>
> >
> > This commit is made of code from the sequencer GSoC project:
> >
> > git://repo.or.cz/git/sbeyer.git
> >
> > (commit 5a78908b70ceb5a4ea9fd4b82f07ceba1f019079)
> >
> > The goal of this commit is to abstract out pick functionnality
> > into a new pick() function made of code from "builtin-revert.c".
> >
> > The new pick() function is in a new "pick.c" file with an
> > associated "pick.h".
>
> Is it pick() or pick_commit()?

It is pick_commit(). Sorry I didn't properly update the commit message.

> By the way, this comment applies to other messages in the series, but do
> these original commits by Stephan lack any meaningful messages?

Well, in Stephan's repo the commit 
(94a568a78d243d7a6c13778bc6b7ac1eb46e48cc) that created the pick.c and 
pick.h files had this message:

------
Libify cherry-pick and revert.

A poor start.
------

(Note that I have seen far worse commit messages in other GSoC students' 
repo.)
And then quite a few other commits improved on this one, and the whole gives 
the code that is in this patch.

> I would 
> actually have expected to see a commit log message that looks more like:
>
> 	From: Stephan Beyer <...>
>
>         Meaningful log message Stephan describes what this commit does in
>         sufficient detail, perhaps with clarification and rewording by
>         Christian.
>
> 	Second and subsequent paragraph to explain it as necessary.
>
> 	[cc: this is based on 5a78908b70ceb5a4ea9fd4b82f07ceba1f019079
>         from Stephan's git://repo.or.cz/git/sbeyer.git]
>
>         Signed-off-by: ...
>
> The point is that the "origin info" would be a side-note, not the first
> and the most important thing in the message.

I tried to improve some commit messages like this in the v3 that will follow 
this email.

Thanks,
Christian.