From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mon, 06 Oct 2008 14:13:17 GMT
Subject: Re: [PATCH] Use "git_config_string" to simplify "remote.c" code in "handle_config"
Message-ID: <alpine.DEB.1.00.0810061610400.22125@pacific.mpi-cbg.de.mpi-cbg.de>
In-Reply-To: <20081003033937.GA11594@eratosthenes.cryptobackpack.org>

Hi,

On Thu, 2 Oct 2008, David Bryson wrote:

> 
> Signed-off-by: David Bryson <david@statichacks.org>
> 
> I tried to keep with the naming/coding conventions that I found in
> remote.c.  Feedback welcome.
> 
> ---

Usually this comment goes after the --- but other than that, the form is 
as perfect as you can wish for.

> @@ -314,15 +315,15 @@ static int handle_config(const char *key, const char *value, void *cb)
>  			return 0;
>  		branch = make_branch(name, subkey - name);
>  		if (!strcmp(subkey, ".remote")) {
> -			if (!value)
> -				return config_error_nonbool(key);
> -			branch->remote_name = xstrdup(value);
> +			if (git_config_string(&v, key, value) ) 
> +				return -1;
> +			branch->remote_name = v;

What is the reason not to write

			if (git_config_string(&branch->remote_name, key, value))
				return -1;

?  (Also note that we do not like the space between the two closing 
parentheses.)

Ciao,
Dscho