From: Kaartic Sivaraam Date: Sun, 08 Aug 2021 17:41:29 GMT Subject: Re: [GSoC] [PATCH v4 1/8] submodule--helper: add options for compute_submodule_clone_url() Message-ID: <0752736f-11d5-103b-653f-a4bbe6436304@gmail.com> In-Reply-To: <20210807071613.99610-2-raykar.ath@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 > Mentored-by: Christian Couder > Mentored-by: Shourya Shukla Otherwise, this looks good. -- Sivaraam