Re: [PATCHv4] Make git-clone respect branch.autosetuprebase

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 6, 2009, 05:29 UTC
Message-ID: <7vsklrw75o.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1236308321-13557-1-git-send-email-pknotz@sandia.gov>

"Pat Notz" <pknotz@sandia.gov> writes:

> Signed-off-by: Pat Notz <pknotz@sandia.gov>

Thanks. I do not think there is substantial difference from the one I queued in 'pu', so I'd keep that one instead, but here are a few comments for future reference.

Show 10 quoted lines

> diff --git a/branch.c b/branch.c
> index 1f00e44..332223b 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -32,21 +32,52 @@ static int find_tracked_branch(struct remote *remote, void *priv)
>  	return 0;
>  }
>  
> -static int should_setup_rebase(const struct tracking *tracking)
> +static int should_setup_rebase(const char * origin)

"const char *origin";

Show 5 quoted lines

> +void install_branch_config(int verbose_flag,
> +			   const char *local,
> +			   const char *origin,
> +			   const char *remote)
> +{

I called the parameter "flag" not "verbose" very much on purpose; we can introduce flags that control aspects other than verbosity if we wanted to later. Renaming it to verbose_flag defeats the extensibility.

Show 13 quoted lines

> diff --git a/branch.h b/branch.h
> index 9f0c2a2..9f7fdb0 100644
> --- a/branch.h
> +++ b/branch.h
> @@ -21,4 +21,15 @@ 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"
> + * taken from origin "origin".
> + */
> +#define BRANCH_CONFIG_QUIET   00

This QUIET is also against the extensible design, as later flags may mean things unrelated to verbosity. With bitfield "flags" parameter, an unset bit simply means "do not trigger this feature" and it is customary to pass 0 to such a function when the caller does not want anything special.