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
