Re: [PATCH] revert.c: Allow to specify -x via git-config

[Jonathan Nieder <jrnieder@gmail.com>]

Hi,

Guido Günther wrote:

> Without this when maintaining stable branches it's easy to forget to use
> -x to track where a patch was cherry-picked from.
[...]
> --- a/Documentation/git-cherry-pick.txt
> +++ b/Documentation/git-cherry-pick.txt
> @@ -215,6 +215,14 @@ the working tree.
>  spending extra time to avoid mistakes based on incorrectly matching
>  context lines.
>  
> +CONFIGURATION
> +-------------
> +
> +See linkgit:git-config[1] for core variables.
> +
> +cherrypick.record-origin::
> +	Default for the `-x` option. Defaults to `false`.

I'm not convinced this is a good idea.  Even if I always want -x when
cherry-picking to stable, isn't this going to add the extra clutter
line when I cherry-pick on other branches?  It's especially worrying
because there would be no way to override the configuration with a
flag on the command line.  ("-r" which used to do that is now a
no-op.)

I would be more easily convinced by a '[branch "foo"]
recordcherrypickorigins' option that makes cherry-pick default to '-x'
when and only when on the "foo" branch.

Can you say more about the context?  Why is it important to record the
original commit id?  Is it a matter of keeping a reminder of the
commits' similarity (which cherry-pick without '-x' does ok by reusing
the same message) or are people reviewing the change downstream going
to be judging the change based on the recorded upstream commit id?
(Like linux's stable-<version> branches --- but those have other
requirements so I don't think this configuration would work as is
there.)

[...]
> +++ b/builtin/revert.c
> @@ -196,6 +196,15 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
[...]
> +	if (!strcmp(var, "cherrypick.record-origin"))
> +		opts->record_origin = git_config_bool (var, value);

More nitpicky: git uses camelCase, not dash-delimited, for multiword
configuration items.  The config parsing machinery normalizes to
lowercase, so this would then be "cherrypick.recordorigin".

Hope that helps,
Jonathan