Re: [PATCH] commit: Add -f, --fixes <commit> option to add Fixes: line

From: Christian Couder
Date: Sun Oct 27 2013 - 15:11:01 EST


[Sorry I already sent the reply below to Johan only instead of everyone.]

Hi Johan,

On Sun, Oct 27, 2013 at 11:59 AM, Johan Herland <johan@xxxxxxxxxxx> wrote:
> On Sun, Oct 27, 2013 at 10:20 AM, Josh Triplett <josh@xxxxxxxxxxxxxxxx> wrote:
>>
>> ...good suggestion:
>>
>> ~/src/linux$ git log --grep='stable@' --oneline --since='1 year ago' | wc -l
>> 2769
>> ~/src/linux$ git log --grep='stable@' --oneline --since='1 year ago' --pretty=format:%an | sort -u | wc -l
>> 839
>>
>> Several thousand commits per year by hundreds of unique people seems
>> like enough to justify a short option.
>
> I think this can be solved just as well (if not better) using a
> combination of a commit message template (or a prepare-commit-msg
> hook) and a commit-msg hook.

Your suggestion is very good, and it is not incompatible with command
line options.
So both could be implemented and even work together.

For example if "-f ack:Peff" was passed to the command line, "git commit" could
lookup in the commit message template and see if there is one
RFC822-style header
that starts with or contains "ack" (discarding case) and it could look
in some previous commits if
there is an author whose name contains "Peff" (discarding case) and if
it is the case
it could append the following to the bottom of the commit message:

Fixes:
Reported-by:
Suggested-by:
Improved-by:
Acked-by: Jeff King <peff@xxxxxxxx>
Reviewed-by:
Tested-by:
Signed-off-by: Myself <myself@xxxxxxxxxxx>

(I suppose that the sob is automatically added.)

It would work also with "-f fix:security-bug" and would put something
like what you suggested:

Fixes: 1234beef56 (Commit message summmary)

> Then, the commit-msg hook can clean up and transform this into the
> final commit message:
>
> My commit subject
>
> This is the commit message body.
>
> Fixes: 1234beef56 (Commit message summmary)
> Reported-by: Joe User <j.user@xxxxxxxxxxx>
> Improved-by: Joe Hacker <j.hacker@xxxxxxxxxxx>
> Tested-by: Joe Tester <j.tester@xxxxxxxxxxx>
> Signed-off-by: Myself <myself@xxxxxxxxxxx>
>
> Here, the commit-msg hook removes the fields that were not filled in,
> and performs additional filtering on the "Fixes" line (Adding commit
> message summary). The filtering could also resolve ref names, so that
> if you had refs/tags/security-bug pointing at the buggy commit, then:
>
> Fixes: security-bug
>
> would be expanded/DWIMed into:
>
> Fixes: 1234beef56 (Commit message summmary)
>
> Obviously, any other fancy processing you want to do into in the
> commit-msg hook can be done as well, adding footnotes, checking that
> commits are present in the ancestry, etc, etc.

Yeah, the commit message hook could do some more processing if the
user adds or changes stuff.

> Three good reasons to go this way:
>
> 1. If the user forgets to supply command-line options like -s,
> --fixes, etc, there is a nice reminder in the supplied form.

Great!

> 2. No need to add any command-line options to Git.

This is not a good reason. If many users prefer a command line option,
why not let them use that?

> 3. The whole mechanism is controlled by the project. The kernel folks
> can do whatever they want in their templates/hooks without needing
> changes to the Git project.

The Git project already manages sob lines. It would be a good thing if
it could manage
more of this stuff to help users in a generic way while taking care of
user preferences.

Best regards,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/