Matt Rogers <mattr94@xxxxxxxxx> writes: >> for clarity, I think this patch should be split up further. >> >> For example: >> >> - moving an enum and adding a new entry should be avoided > > As far as adding a new entry, that could probably be done in a separate patch. > The submodule scoping is correct (mostly so config options that come from > submodule blobs have a sane value to set the new scope field of > git_config_source), but moving it is unavoidable as I'd either have to move > config_scope or git_config_source for this patch to work, and moving > git_config_source seemed like a ton more work > >> - does the changes to '/config.c' fix something? > > Another thing, that in hindsight could probably be split out. The other changes > do fix the fact that previously recursive calls to do_git_config_sequence() > would blow awway the current_parsing_scope information for higher level callers. > This is not super common, but does occur when --show-scope is used with the > --blob option. > >> - exposing config_scope_name should have been done before PATCH 4/6 already > > If that's better/more convenient, I don't have a problem breaking that > out and moving > it there. Alright. Thanks for another round of review.
