From: Stefan Beller <sbeller@google.com>
To: Antonio Ospite <ao2@ao2.it>
Cc: git <git@vger.kernel.org>, Brandon Williams <bmwill@google.com>,
Daniel Graña <dangra@gmail.com>,
Jonathan Nieder <jrnieder@gmail.com>,
Richard Hartmann <richih.mailinglist@gmail.com>
Subject: Re: [RFC PATCH 01/10] config: make config_from_gitmodules generally useful
Date: Mon, 14 May 2018 18:05:19 -0700
Message-ID: <CAGZ79kag=1h506FGg72_F9Rmz4nqPN19kaywfTtD3WnNWnxD9w@mail.gmail.com> (raw)
In-Reply-To: <20180514105823.8378-2-ao2@ao2.it>
On Mon, May 14, 2018 at 3:58 AM, Antonio Ospite <ao2@ao2.it> 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.