Re: [GSoC] [PATCH v4 1/8] submodule--helper: add options for compute_submodule_clone_url()

[Kaartic Sivaraam <kaartic.sivaraam@gmail.com>]

On 07/08/21 12:46 pm, Atharva Raykar wrote:
> Let's modify the interface to `compute_submodule_clone_url()` function
> by adding two more arguments, so that we can reuse this in various parts
> of `submodule--helper.c` that follow a common pattern, which is--read
> the remote url configuration of the superproject and then call
> `relative_url()`.
> 
> This function is nearly identical to `resolve_relative_url()`, the only
> difference being the extra warning message. We can add a quiet flag to
> it, ...

It took me a while to figure what "it" meant in the above sentence. Does it
refer to `compute_submodule_clone_url` or `resolve_relative_url`. After one
sees the patch and takes a look at `resolve_relative_url`, it's clear the "it"
indeed does refer to `resolve_relative_url`. But it might worth clarifying this
in the commit message itself.

Certainly not worth a re-roll on its own. May be Junio could amend this while queing ?


> ...  to suppress that warning when not needed, and then refactor
> `resolve_relative_url()` by using this function, something we will do in
> the next patch.
> 
> Having this functionality factored out will be useful for converting the
> rest of `submodule add` in subsequent patches.
> 
> Signed-off-by: Atharva Raykar <raykar.ath@gmail.com>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Shourya Shukla <periperidip@gmail.com>

Otherwise, this looks good.

-- 
Sivaraam