Re: [PATCH 0/4] config.h

From: Jeff King <peff@peff.net>
Date: Jun 12, 2017, 21:45 UTC
Message-ID: <20170612214502.xynyfswj2ipf25ah@sigill.intra.peff.net>
In-Reply-To: <20170612213406.83247-1-bmwill@google.com>

On Mon, Jun 12, 2017 at 02:34:02PM -0700, Brandon Williams wrote:

Show 12 quoted lines

> 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).

-Peff