Re: [PATCH] Add option -b/--branch to clone for select a new HEAD

From: Jeff King <peff@peff.net>
Date: Aug 25, 2009, 01:57 UTC
Message-ID: <20090825015726.GB7655@coredump.intra.peff.net>
In-Reply-To: <1251146568-25248-1-git-send-email-catap@catap.ru>

On Tue, Aug 25, 2009 at 12:42:48AM +0400, Kirill A. Korinskiy wrote:

Show 5 quoted lines

> Sometimes (especially on production systems) we need to use only one
> remote branch for building software. It really annoying to clone
> origin and then swith branch by hand everytime. So this patch provide
> functionality to clone remote branch with one command without using
> checkout after clone.

If you are doing this a lot, it is probably a sign that you should repoint the "HEAD" of the parent repository.

That being said, you may want one branch half the time, and another branch the other half. So I think this is a good feature.

A few comments:

> ---
>  Documentation/git-clone.txt |    4 ++++
>  builtin-clone.c             |   26 +++++++++++++++++++++++---
>  2 files changed, 27 insertions(+), 3 deletions(-)

Tests?

> -	const struct ref *refs, *head_points_at, *remote_head, *mapped_refs;
> -	struct strbuf key = STRBUF_INIT, value = STRBUF_INIT;
> +	const struct ref *refs, *head_points_at, *remote_head = NULL, *mapped_refs;
> +	struct strbuf key = STRBUF_INIT, value = STRBUF_INIT, branch_head = STRBUF_INIT;

Style nit: I don't know if we have a style guideline for declaring variables, but I find these "many variables on a line" declarations annoying for reviewing, since it is hard to see what actually changed (and yes, you only added a declaration on one, so I am partially complaining about the person who came before you :) ).

> +		if (option_branch)
> +			die("--bare and --branch %s options are incompatible.",
> +			    option_branch);

Hmm. Would it perhaps make sense to have "--bare --branch foo" point the HEAD of the newly created bare repo, but not impact the (nonexistent) working tree?

-Peff