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@xxxxxxxxx> Mentored-by: Christian Couder <christian.couder@xxxxxxxxx> Mentored-by: Shourya Shukla <periperidip@xxxxxxxxx>
Otherwise, this looks good. -- Sivaraam
