Re: [PATCH 01/11] refs-internal.c: make files_log_ref_write() static

From: Ramsay Jones <ramsay@ramsayjones.plus.com>
Date: Feb 13, 2017, 20:14 UTC
Message-ID: <6510f4b2-51bb-eabd-9cd7-30bc4164b25d@ramsayjones.plus.com>
In-Reply-To: <20170213152011.12050-2-pclouds@gmail.com>

On 13/02/17 15:20, Nguyễn Thái Ngọc Duy wrote:

Show 20 quoted lines

> Created in 5f3c3a4e6f (files_log_ref_write: new function - 2015-11-10)
> but probably never used outside refs-internal.c
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  refs/files-backend.c | 3 +++
>  refs/refs-internal.h | 4 ----
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index cdb6b8ff5..75565c3aa 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -165,6 +165,9 @@ static struct ref_entry *create_dir_entry(struct files_ref_store *ref_store,
>  					  const char *dirname, size_t len,
>  					  int incomplete);
>  static void add_entry_to_dir(struct ref_dir *dir, struct ref_entry *entry);
> +static int files_log_ref_write(const char *refname, const unsigned char *old_sha1,
> +			       const unsigned char *new_sha1, const char *msg,
> +			       int flags, struct strbuf *err);

Why? I don't like adding forward declarations unless it is absolutely necessary (ie mutually recursive functions), and even in the current 'pu' branch (@c04899d50), the definition of this function appears before all uses in this file. (ie, just add static to the definition).

What am I missing?

ATB, Ramsay Jones

Show 19 quoted lines

>  
>  static struct ref_dir *get_ref_dir(struct ref_entry *entry)
>  {
> diff --git a/refs/refs-internal.h b/refs/refs-internal.h
> index 33adbf93b..59e65958a 100644
> --- a/refs/refs-internal.h
> +++ b/refs/refs-internal.h
> @@ -222,10 +222,6 @@ struct ref_transaction {
>  	enum ref_transaction_state state;
>  };
>  
> -int files_log_ref_write(const char *refname, const unsigned char *old_sha1,
> -			const unsigned char *new_sha1, const char *msg,
> -			int flags, struct strbuf *err);
> -
>  /*
>   * Check for entries in extras that are within the specified
>   * directory, where dirname is a reference directory name including
>