Re: [PATCH 0/4] config.h

[Brandon Williams <bmwill@google.com>]

On 06/12, Jeff King wrote:
> On Mon, Jun 12, 2017 at 02:34:02PM -0700, Brandon Williams wrote:
> 
> > After some discussion I realized that my 'repository object' series was getting
> > to be rather long and increase in scope.  So I've decided to break off these
> > couple patches into their own series so they can be reviewed more easily.  This
> > should also let them be merged in faster as I suspect it'll take a while for my
> > 'repository object' series to be reviewed thourouly and these couple patches
> > could result in a lot of merge conflicts as it touches a lot of files.
> > 
> > Brandon Williams (4):
> >   config: create config.h
> >   config: remove git_config_iter
> >   config: don't include config.h by default
> >   config: don't implicitly use gitdir
> 
> These all seem reasonable to me. Patch 3 made me shrug a little, because
> it seems like the majority of C files end up including it anyway. I
> suspect you could break config.h down into two files: the few things
> that everybody needs (git_config() and the few parsing functions needed
> in callbacks) and the ones for commands that actually manipulate the
> config.
> 
> That would reduce the surface area of the module that most callers look
> at, but I don't think there's a huge benefit to doing so (mostly it just
> makes re-compiling faster by decreasing the chance that a dependent
> header has changed for each file).

Yes, ultimately I think it would be a good thing to break config.c down
into at least 2 more files (the file parsing logic and the config_set
logic) but that can be done at a later point.  I started looking at
doing that now but that logic is a little more entangled than I thought
it was.

Thanks for taking a look Jeff!

-- 
Brandon Williams