Re: [PATCH] revert.c: Allow to specify -x via git-config
On Tue, Feb 18, 2014 at 07:56:20AM +0100, 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. Signed-off-by: Guido Günther a...@sigxcpu.org --- Documentation/git-cherry-pick.txt | 8 builtin/revert.c | 10 ++ 2 files changed, 18 insertions(+) diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt index c205d23..c35064f 100644 --- 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:: For consistency, this should be cherrypick.recordOrigin, although I wonder if we should permit cherry-pick here so as to keep the config section the same as the name of the command. I think this should also be added to git-config(1) + Default for the `-x` option. Defaults to `false`. + SEE ALSO linkgit:git-revert[1] diff --git a/builtin/revert.c b/builtin/revert.c index 87659c9..df9718f 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -196,6 +196,15 @@ int cmd_revert(int argc, const char **argv, const char *prefix) return res; } +static int git_cherry_pick_config(const char *var, const char *value, void *cb) +{ + struct replay_opts *opts = cb; + + if (!strcmp(var, cherrypick.record-origin)) + opts-record_origin = git_config_bool (var, value); + return 0; If you change this to: return git_default_config(var, value, cb); then the code below won't end up walking the config twice. +} + int cmd_cherry_pick(int argc, const char **argv, const char *prefix) { struct replay_opts opts; @@ -204,6 +213,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix) memset(opts, 0, sizeof(opts)); opts.action = REPLAY_PICK; git_config(git_default_config, NULL); + git_config(git_cherry_pick_config, opts); parse_args(argc, argv, opts); res = sequencer_pick_revisions(opts); if (res 0) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] revert.c: Allow to specify -x via git-config
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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] revert.c: Allow to specify -x via git-config
On Tue, Feb 18, 2014 at 09:49:13AM -0800, Jonathan Nieder wrote: 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.) I can provide a use case. At work, we merge into the maintenance and development branches and cherry-pick from the maintenance to the stable branches. We want committers to always use -x -s because we need to know which reviewer backported the change and we want to be able to track which commits have been backported and whether any reverts also need to be cherry-picked. We also have automated tools that want this information. I usually solve this with an alias (backport = cherry-pick -x -s), but I can see how this might be a useful option. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature