From: David Turner Date: Mon, 09 Nov 2015 21:23:55 GMT Subject: Re: [PATCH v7 07/11] refs/refs-internal.h: new header file Message-ID: <1447104235.20147.1.camel@twopensource.com> In-Reply-To: <5640F823.5010902@ramsayjones.plus.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 > > --- > > 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.