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

[Johan Herland]

On Sun, Oct 27, 2013 at 10:20 AM, Josh Triplett <josh@xxxxxxxxxxxxxxxx> wrote:
> On Sun, Oct 27, 2013 at 09:09:32AM +0100, Thomas Rast wrote:
>> Josh Triplett <josh@xxxxxxxxxxxxxxxx> writes:
>> > On Sun, Oct 27, 2013 at 06:42:44AM +0100, Michael Haggerty wrote:
>> >> But I don't think that this feature should be given the "-f" short
>> >> option, as (a) -f often means "force"; (b) it will increase the
>> >> confusion with --fixup; (c) it just doesn't strike me as being likely to
>> >> be such a frequently-used option (though if this changes over time the
>> >> "-f" option could always be granted to it later).
>> >
>> > (a) -n often means --dry-run, but for commit it means --no-verify.
>> > Different commands have different options, and commit doesn't have a
>> > --force to abbreviate as -f.
>> >
>> > (b) If anything, I think the existence of a short option will make the
>> > distinction more obvious, since -f and --fixup are much less similar
>> > than --fixes and --fixup.  Most users will never type --fixes, making
>> > confusion unlikely.
>> >
>> > (c) Short option letters tend to be first-come first-serve unless
>> > there's a strong reason to do otherwise.  Why reserve 'f' for some
>> > hypothetical future option that doesn't exist yet?
>>
>> No, lately the direction in Git has been to avoid giving options a
>> one-letter shorthand until they have proven so useful that people using
>> it in the wild start to suggest that it should have one.
>>
>> See e.g.
>>
>>   http://article.gmane.org/gmane.comp.version-control.git/233998
>>   http://article.gmane.org/gmane.comp.version-control.git/168748
>
> Fair enough; easy enough to drop -f if that's the consensus.  However...
>
>> A much better argument would be if it was already clear from the specs
>> laid out for Fixes that n% of the kernel commits will end up having this
>> footer, and thus kernel hackers will spend x amount of time spelling out
>> --fixes and/or confusing it with --fixup to much headache.
>
> ...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.

The former appends a section of commonly-used RFC822-style headers
(with empty values) to the bottom of the commit message, e.g. some
variation on this:

  Fixes:
  Reported-by:
  Suggested-by:
  Improved-by:
  Acked-by:
  Reviewed-by:
  Tested-by:
  Signed-off-by:

Then the user (in addition to writing the commit message above this
block) may choose to fill in one or more values in this "form", e.g.
like this:

  My commit subject

  This is the commit message body.

  Fixes: 1234beef
  Reported-by: Joe User <j.user@xxxxxxxxxxx>
  Suggested-by:
  Improved-by: Joe Hacker <j.hacker@xxxxxxxxxxx>
  Acked-by:
  Reviewed-by:
  Tested-by: Joe Tester <j.tester@xxxxxxxxxxx>
  Signed-off-by: Myself <myself@xxxxxxxxxxx>

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.

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.

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

 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.


...Johan

-- 
Johan Herland, <johan@xxxxxxxxxxx>
www.herland.net
--
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/