Re: [PATCH 06/10] Change incorrect "remote branch" to "remote tracking branch" in C code

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Oct 28, 2010, 19:13 UTC
Message-ID: <20101028191303.GE14212@burratino>
In-Reply-To: <1288290117-6734-7-git-send-email-Matthieu.Moy@imag.fr>

Matthieu Moy wrote:

Show 11 quoted lines

> --- a/branch.h
> +++ b/branch.h
> @@ -22,7 +22,7 @@ void create_branch(const char *head, const char *name, const char *start_name,
>  void remove_branch_state(void);
>  
>  /*
> - * Configure local branch "local" to merge remote branch "remote"
> + * Configure local branch "local" to merge remote-tracking branch "remote"
>   * taken from origin "origin".
>   */
>  #define BRANCH_CONFIG_VERBOSE 01

This still seems inaccurate (and even misleading).  Presumably you mean

	             ... to merge remote-tracking branch "origin/remote"
	based on origin "origin".

or

	             ... to merge remote branch "remote"
	taken from origin "origin"

or even better,

	Configure local branch "local" as downstream to branch "remote"
	from remote "origin".  Used by git branch --set-upstream.

More confusion could presumably be avoided by renaming the parameters.

Show 9 quoted lines

> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -359,7 +359,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
>  			what = rm->name + 10;
>  		}
>  		else if (!prefixcmp(rm->name, "refs/remotes/")) {
> -			kind = "remote branch";
> +			kind = "remote-tracking branch";
>  			what = rm->name + 13;

I didn't follow before --- what concrete effect does this have? Are scripts affected? Is fmt-merge-msg affected?

Show 14 quoted lines

> --- a/builtin/remote.c
> +++ b/builtin/remote.c
> @@ -791,9 +791,9 @@ static int rm(int argc, const char **argv)
>  
>  	if (skipped.nr) {
>  		fprintf(stderr, skipped.nr == 1 ?
> -			"Note: A non-remote branch was not removed; "
> +			"Note: A branch outside the refs/remotes/ hierarchy was not removed;\n"
>  			"to delete it, use:\n" :
> -			"Note: Non-remote branches were not removed; "
> +			"Note: Some branches outside the refs/remotes/ hierarchy were not removed;\n"
>  			"to delete them, use:\n");
>  		for (i = 0; i < skipped.nr; i++)
>  			fprintf(stderr, "  git branch -d %s\n",

Nice.

Maybe the commit message should explain the reasoning, something like:

 The non-deleted branch is somehow remote-tracking since "fetch" feeds
 it, and the code detecting this case is:

	/* don't delete non-remote-tracking refs */
	if (prefixcmp(refname, "refs/remotes")) {
		/* advise user how to delete local branches */
		if (!prefixcmp(refname, "refs/heads/"))
			string_list_append(branches->skipped,
					   abbrev_branch(refname));
		/* silently skip over other non-remote refs */
		return 0;
	}
 
 hence, really "branches outside the refs/remotes/ hierarchy".

Thanks for your tireless work.  Everything not mentioned above looks good.