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

[David Turner <dturner@twopensource.com>]

On Mon, 2015-11-09 at 19:46 +0000, Ramsay Jones wrote:
> 
> 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

+1 on this.