Re: [PATCH 25/29] refs: resolve symbolic refs first

From: David Turner <dturner@twopensource.com>
Date: Apr 28, 2016, 23:40 UTC
Message-ID: <1461886803.4123.14.camel@twopensource.com>
In-Reply-To: <a33272db057ac6327cc0c87c002473c89ffcca6b.1461768690.git.mhagger@alum.mit.edu>

On Wed, 2016-04-27 at 18:57 +0200, Michael Haggerty wrote: +retry: ...

> +		if (--attempts_remaining > 0)
> +			goto retry;

could this be a loop instead of using gotos?

Show 14 quoted lines

> +			/*
> +			 * There is a directory in the way,
> +			 * but we	 don't know of any references
> +			 * that it should contain. This might
> +			 * be a directory that used to contain
> +			 * references but is now empty. Try to
> +			 * remove it; otherwise it might cause
> +			 * trouble when we try to rename the
> +			 * lockfile into place.
> +			 */
> +			strbuf_addf(err, "there is a non-empty directory '%s' "
> +				          "blocking reference '%s'",
> +					  ref_file.buf,refname);
> +			goto error_return;

We don't actually try to remove anything here, so that comment seems wrong?