Re: [PATCH v7 07/11] refs/refs-internal.h: new header file

[Ramsay Jones <ramsay@ramsayjones.plus.com>]

On 09/11/15 17:03, Michael Haggerty wrote:
> There are a number of constants, structs, and static functions defined
> in refs.c and treated as private to the references module. But we want
> to support multiple reference backends within the reference module,
> and those backends will need access to some heretofore private
> declarations.
> 
> We don't want those declarations to be visible to non-refs code, so we
> don't want to move them to refs.h. Instead, add a new header file,
> refs/refs-internal.h, that is intended to be included only from within
> the refs module. Make some functions non-static and move some
> declarations (and their corresponding docstrings) from refs.c to this
> file.
> 
> In a moment we will add more content to the "refs" subdirectory.
> 
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
>  refs.c               | 175 +++----------------------------------------------
>  refs/refs-internal.h | 182 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 191 insertions(+), 166 deletions(-)
>  create mode 100644 refs/refs-internal.h
> 
> diff --git a/refs.c b/refs.c
> index f48c58a..9aff0c8 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1,6 +1,6 @@
>  #include "cache.h"
> +#include "refs/refs-internal.h"
>  #include "lockfile.h"
> -#include "refs.h"

This looked wrong to me, until I had read the remainder of the
patch and noticed that the 'internal' header #included the
'public' interface header.

Unfortunately, this still feels wrong to me! I would rather that
the internal header _not_ include the public header (so, include
them _both_ when necessary). Just my opinion, which you can simply
ignore. :-D

ATB,
Ramsay Jones