On Mon, May 14, 2018 at 3:58 AM, Antonio Ospite <ao2@xxxxxx> wrote: > The config_from_gitmodules() function is a good candidate for > a centralized point where to read the gitmodules configuration file. It is very tempting to use that function. However it was introduced specifically to not do that. ;) See the series that was merged at 5aa0b6c506c (Merge branch 'bw/grep-recurse-submodules', 2017-08-22), specifically f20e7c1ea24 (submodule: remove submodule.fetchjobs from submodule-config parsing, 2017-08-02), where both builtin/fetch as well as the submodule helper use the pattern to read from the .gitmodules file va this function and then overlay it with the read from config. > Add a repo argument to it to make the function more general, and adjust > the current callers in cmd_fetch and update-clone. This could be a preparatory patch, but including it here is fine, too. > As a proof of the utility of the change, start using the function also > in repo_read_gitmodules which was basically doing the same operations. I think they were separated for the reason outlined above, or what Brandon said in his reply. I think extending 'repo_read_gitmodules' makes sense, as that is used everywhere for the loading of submodule configuration.
