From: Junio C Hamano <junkio@cox.net>
Date: Mon, 18 Dec 2006 07:08:23 GMT
Subject: Re: [PATCH] git-branch: deleting remote branches in new layout
Message-ID: <7vbqm13cm0.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <45863044.4040406@gmail.com>

Quy Tonthat <qtonthat@gmail.com> writes:

> Now that remote branches are in refs/remotes/, branch -D needs to know
> where to find them.
>
> Signed-off-by: Quy Tonthat <qtonthat@gmail.com>

I recognize that giving end users a way to remove a "remote
tracking branch" might be a worthy goal ("update-ref -d" _could_
be used, but "branch -D" feels more natural).

> +	char ** s;
> +	static char * branches_dir[] = {"refs/heads", "refs/remotes", NULL};

But I do not like these two entries in branches_dir[].

(style: lose SP after '*' in these two lines, by the way).

If you had refs/heads/$X and refs/remotes/$X, I do not think
this code allows you to disambiguate.  You cannot remove
"remote" one without first removing the local one, can you?

> +		for (s = branches_dir; *s != NULL; s++) {
> +			name = xstrdup(mkpath("%s/%s", *s, argv[i]));
> +			if (resolve_ref(name, sha1, 1, NULL))
> +				break;
> +		}
> +		if (*s == NULL)
>  			die("Branch '%s' not found.", argv[i]);

(style: we seem to prefer "if (!*s)").