public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Ramkumar Ramachandra <artagnon@gmail•com>
Cc: Git List <git@vger•kernel.org>, Jeff King <peff@peff•net>,
	Junio C Hamano <gister@pobox•com>
Subject: Re: [RFC/PATCH] format-patch: introduce branch.*.forkedFrom
Date: Tue, 07 Jan 2014 12:36:45 -0800	[thread overview]
Message-ID: <xmqq8uurcyw2.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1389126588-3663-1-git-send-email-artagnon@gmail.com> (Ramkumar Ramachandra's message of "Wed, 8 Jan 2014 01:59:48 +0530")

Ramkumar Ramachandra <artagnon@gmail•com> writes:

> A very common workflow for preparing patches involves working off a
> topic branch and generating patches against 'master' to send off to the
> maintainer. However, a plain
>
>   $ git format-patch -o outgoing
>
> is a no-op on a topic branch, and the user has to remember to specify
> 'master' explicitly everytime. This problem is not unique to
> format-patch; even a
>
>   $ git rebase -i
>
> is a no-op because the branch to rebase against isn't specified.
>
> To tackle this problem, introduce branch.*.forkedFrom which can specify
> the parent branch of a topic branch. Future patches will build
> functionality around this new configuration variable.
>

I do not mind allowing laziness by defaulting to something, but I am
not enthused by an approach that adds the new variable whose value
is questionable.  The description does not justify at all why
@{upstream} is not a good default (unless the workflow is screwed up
and @{upstream} is set to point at somewhere that is _not_ a true
upstream, that is).

> Cc: Jeff King <peff@peff•net>
> Cc: Junio C Hamano <gister@pobox•com>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail•com>
> ---
>  Since -M, -C, -D are left in the argc, checking argc < 2 isn't
>  sufficient.
>
>  I wanted to get an early reaction before wiring up checkout and
>  rebase.
>
>  But I wanted to discuss the overall idea of the patch.
>  builtin/log.c           | 21 +++++++++++++++++++++
>  t/t4014-format-patch.sh | 20 ++++++++++++++++++++
>  2 files changed, 41 insertions(+)
>
> diff --git a/builtin/log.c b/builtin/log.c
> index b97373d..525e696 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -674,6 +674,7 @@ static int thread;
>  static int do_signoff;
>  static const char *signature = git_version_string;
>  static int config_cover_letter;
> +static const char *config_base_branch;
>  
>  enum {
>  	COVER_UNSET,
> @@ -750,6 +751,22 @@ static int git_format_config(const char *var, const char *value, void *cb)
>  		config_cover_letter = git_config_bool(var, value) ? COVER_ON : COVER_OFF;
>  		return 0;
>  	}
> +	if (starts_with(var, "branch.")) {
> +		const char *name = var + 7;
> +		const char *subkey = strrchr(name, '.');
> +		struct strbuf buf = STRBUF_INIT;
> +
> +		if (!subkey)
> +			return 0;
> +		strbuf_add(&buf, name, subkey - name);
> +		if (branch_get(buf.buf) != branch_get(NULL))
> +			return 0;
> +		strbuf_release(&buf);
> +		if (!strcmp(subkey, ".forkedfrom")) {
> +			if (git_config_string(&config_base_branch, var, value))
> +				return -1;
> +		}
> +	}
>  
>  	return git_log_config(var, value, cb);
>  }
> @@ -1324,6 +1341,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>  		die (_("--subject-prefix and -k are mutually exclusive."));
>  	rev.preserve_subject = keep_subject;
>  
> +	if (argc < 2 && config_base_branch) {
> +		argv[1] = config_base_branch;
> +		argc++;
> +	}
>  	argc = setup_revisions(argc, argv, &rev, &s_r_opt);
>  	if (argc > 1)
>  		die (_("unrecognized argument: %s"), argv[1]);
> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> index 73194b2..2ea94af 100755
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -1370,4 +1370,24 @@ test_expect_success 'cover letter auto user override' '
>  	test_line_count = 2 list
>  '
>  
> +test_expect_success 'branch.*.forkedFrom matches' '
> +	mkdir -p tmp &&
> +	test_when_finished "rm -rf tmp;
> +		git config --unset branch.rebuild-1.forkedFrom" &&
> +
> +	git config branch.rebuild-1.forkedFrom master &&
> +	git format-patch -o tmp >list &&
> +	test_line_count = 2 list
> +'
> +
> +test_expect_success 'branch.*.forkedFrom does not match' '
> +	mkdir -p tmp &&
> +	test_when_finished "rm -rf tmp;
> +		git config --unset branch.foo.forkedFrom" &&
> +
> +	git config branch.foo.forkedFrom master &&
> +	git format-patch -o tmp >list &&
> +	test_line_count = 0 list
> +'
> +
>  test_done

  parent reply	other threads:[~2014-01-07 20:36 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-07 20:29 [RFC/PATCH] format-patch: introduce branch.*.forkedFrom Ramkumar Ramachandra
2014-01-07 20:30 ` Ramkumar Ramachandra
2014-01-07 20:40   ` Jeff King
2014-01-07 20:48     ` Junio C Hamano
2014-01-07 21:02     ` Ramkumar Ramachandra
2014-01-07 21:16       ` Jeff King
2014-01-07 21:35         ` Ramkumar Ramachandra
2014-01-08  9:33           ` [RFC/PATCH 0/5] <branch>@{publish} shorthand Jeff King
2014-01-08  9:34             ` [PATCH 1/5] sha1_name: refactor upstream_mark Jeff King
2014-01-08  9:34             ` [PATCH 2/5] interpret_branch_name: factor out upstream handling Jeff King
2014-01-08 12:37               ` Ramkumar Ramachandra
2014-01-08  9:35             ` [PATCH 3/5] branch_get: return early on error Jeff King
2014-01-08  9:35             ` [PATCH 4/5] branch_get: provide per-branch pushremote pointers Jeff King
2014-01-08 10:27               ` Jeff King
2014-01-08 10:47                 ` [PATCH] t5531: further "matching" fixups Jeff King
2014-01-10 23:34                   ` Junio C Hamano
2014-01-11  4:22                     ` Jeff King
2014-01-08 11:09                 ` [PATCH 4/5] branch_get: provide per-branch pushremote pointers Jeff King
2014-01-08  9:37             ` [PATCH 5/5] implement @{publish} shorthand Jeff King
2014-01-08 23:42               ` Junio C Hamano
2014-01-09 18:20                 ` Jeff King
2014-01-09 21:24                   ` Junio C Hamano
2014-01-09  8:39               ` Philip Oakley
2014-01-09 22:03                 ` Jeff King
2014-01-09 22:24                 ` Junio C Hamano
2014-01-24  0:16               ` Junio C Hamano
2014-01-24 21:35                 ` Jeff King
2014-01-24 22:05                   ` Ramkumar Ramachandra
2014-01-24 23:12                     ` Junio C Hamano
2014-02-15 11:50                   ` Philip Oakley
2014-02-18  8:52                     ` Jeff King
2014-02-18 13:10                       ` Johan Herland
2014-02-18 19:52                       ` Junio C Hamano
2014-01-08 12:40             ` [RFC/PATCH 0/5] <branch>@{publish} shorthand Ramkumar Ramachandra
2014-01-07 20:36 ` Junio C Hamano [this message]
2014-01-07 20:40   ` [RFC/PATCH] format-patch: introduce branch.*.forkedFrom Ramkumar Ramachandra
2014-01-07 20:42     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq8uurcyw2.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=artagnon@gmail$(echo .)com \
    --cc=gister@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=peff@peff$(echo .)net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox