Glen Choo wrote: > Glen Choo <chooglen@xxxxxxxxxx> writes: > >> I think it would be better to pass a "struct repository" arg to >> config_with_options() instead of mocking a config_source to hold a .repo >> member. > > The flipside is that this would be redundant with an existing use of > git_config_source.repo, so for consistency, we should probably remove > git_config_source.repo. There's only one user of git_config_source.repo > - reading .gitmodules from a blob. It probably made sense to add .repo > the time, but now that we have a second, different use of "struct > repository", accepting an arg is probably better. Agreed, I'd much prefer having a single 'struct repository' instance used in the context of config parsing (if they ever had different values, it probably wouldn't be immediately obvious for debugging). This and all of your other recommendations seem reasonable to me, I'll re-roll shortly. Thanks for the review!
