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

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Feb 18, 2014, 17:49 UTC
Message-ID: <20140218174913.GB7855@google.com>
In-Reply-To: <20140218065620.GA3448@bogon.m.sigxcpu.org>

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.

[...]

Show 13 quoted lines

> --- 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