Subject: Re: [PATCH 02/13] submodule--helper: get remote names from any repository

From: Atharva Raykar <[email-masked]>
Date: Tue, 07 Sep 2021 19:03:48 +0530

Ævar Arnfjörð Bjarmason <[email-masked]> writes:

> On Tue, Sep 07 2021, Atharva Raykar wrote:
>
>> `get_default_remote()` retrieves the name of a remote by resolving the
>> refs from of the current repository's ref store.
>>
>> Thus in order to use it for retrieving the remote name of a submodule,
>> we have to start a new subprocess which runs from the submodule
>> directory.
>>
>> Let's instead introduce a function called `repo_get_default_remote()`
>> which takes any repository object and retrieves the remote accordingly.
>>
>> `get_default_remote()` is then defined as a call to
>> `repo_get_default_remote()` with 'the_repository' passed to it.
>
> I'd find this easier to follow if this were just squashed into the next
> commit. Both are rather small, but following the context of first adding
> a function, then using it, instead of just adding it, changing the old
> users etc. is harder than just having it in one commit.

I am in two minds about this. I initially had both these changes in one
commit, but Christian suggested I split the changes into a part that
refactors existing code (this commit), and one that introduces a new
helper (the next commit).

I guess I will squash it for now and see how it is received. Maybe
bringing down the "/13" might help get more reviews ;-)