Subject: Re: [PATCH v4 02/26] refs: make repack_without_refs and is_branch public

From: Michael Haggerty <[email-masked]>
Date: Fri, 16 Oct 2015 08:34:41 +0200

On 10/15/2015 09:46 PM, David Turner wrote:
> is_branch was already non-static, but this patch declares it in the
> header.

The commit message no longer reflects the patch.

> Signed-off-by: Ronnie Sahlberg <[email-masked]>
> Signed-off-by: David Turner <[email-masked]>
> Signed-off-by: Junio C Hamano <[email-masked]>
> ---
>  refs.c | 5 +++--
>  refs.h | 2 ++
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/refs.c b/refs.c
> index fe71ea0..84abc82 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2816,8 +2816,9 @@ int pack_refs(unsigned int flags)
>  
>  /*
>   * Rewrite the packed-refs file, omitting any refs listed in
> - * 'refnames'. On error, leave packed-refs unchanged, write an error
> - * message to 'err', and return a nonzero value.
> + * 'refnames'. On error, packed-refs will be unchanged, the return
> + * value is nonzero, and a message about the error is written to the
> + * 'err' strbuf.

^^^ ?

It is preferable for docstrings to be written in imperative form, so in
my opinion this is a step backwards...

...literally. Your "new" version comes from an older version of Git; it
was changed in

    79e4d8a9b8 repack_without_refs(): make function private (2015-06-22)

to the imperative form.

Assuming you are using `git-format-patch` to prepare your patches, it is
always a good idea to read over the prepared email files before sending
them to the ML, to check for bloopers like this.

>   *
>   * The refs in 'refnames' needn't be sorted. `err` must not be NULL.
>   */
> diff --git a/refs.h b/refs.h
> index 7367a7f..8408bef 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -237,6 +237,8 @@ int pack_refs(unsigned int flags);
>  int verify_refname_available(const char *newname, struct string_list *extra,
>  			     struct string_list *skip, struct strbuf *err);
>  
> +extern int is_branch(const char *refname);
> +
>  /*
>   * Flags controlling ref_transaction_update(), ref_transaction_create(), etc.
>   * REF_NODEREF: act on the ref directly, instead of dereferencing
> 

Michael

-- 
Michael Haggerty
[email-masked]