Re: [RFC PATCH 01/10] config: make config_from_gitmodules generally useful
- From
Stefan Beller <sbeller@google.com>
- Date
- May 15, 2018, 01:05 UTC
- Message-ID
- <CAGZ79kag=1h506FGg72_F9Rmz4nqPN19kaywfTtD3WnNWnxD9w@mail.gmail.com>
- 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.