Subject: Re: [PATCH v2 05/12] delete_refs(): improve error message

From: Junio C Hamano <[email-masked]>
Date: Mon, 15 Jun 2015 11:29:00 -0700

Michael Haggerty <[email-masked]> writes:

> Subject: Re: [PATCH v2 05/12] delete_refs(): improve error message

I'd call this "make error message more generic".

> Change the error message from
>
>     Could not remove branch %s
>
> to
>
>     could not remove reference %s
>
> * s/branch/reference/. This change makes sense even for the existing
>   caller, which uses the function to delete remote-tracking
>   branches.

and replace this bullet with something like:

* Originally 'branch -d' was the only caller of this, but as part
  of the refs API, we will allow it to be called on refs that is not
  a branch or a remote-tracking branch.

as calling a remote-tracking branch a 'branch' is not incorrect
per-se.  What would count as true improvement is ...

> * Convert it to lower case, as per our usual convention.

... this one.  If somebody eventually chooses to make the message
finer grained by switching on the prefix refs/{what}, so that the
user can differentiate between branches, remote-tracking branches,
tags, etc., that would also count as improvement as well.

>
> Signed-off-by: Michael Haggerty <[email-masked]>
> ---
>  refs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/refs.c b/refs.c
> index 6f62bd1..5386077 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2831,7 +2831,7 @@ int delete_refs(struct string_list *refnames)
>  		const char *refname = refnames->items[i].string;
>  
>  		if (delete_ref(refname, NULL, 0))
> -			result |= error(_("Could not remove branch %s"), refname);
> +			result |= error(_("could not remove reference %s"), refname);
>  	}
>  
>  	return result;