Re: [PATCH v4 1/3] submodule--helper: refactor module_clone()

[Atharva Raykar <raykar.ath@gmail.com>]

On 15-Jun-2021, at 09:21, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Atharva Raykar <raykar.ath@gmail.com> writes:
> 
>> Separate out the core logic of module_clone() from the flag
>> parsing---this way we can call the equivalent of the `submodule--helper
>> clone` subcommand directly within C, without needing to push arguments
>> in a strvec.
>> ---
> 
> Missing sign-off?

Yes, I missed out all the trailers.

>> builtin/submodule--helper.c | 241 +++++++++++++++++++-----------------
>> 1 file changed, 128 insertions(+), 113 deletions(-)
>> 
>> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
>> index d55f6262e9..321b623d81 100644
>> --- a/builtin/submodule--helper.c
>> +++ b/builtin/submodule--helper.c
>> @@ -1658,45 +1658,20 @@ static int module_deinit(int argc, const char **argv, const char *prefix)
>> 	return 0;
>> }
>> 
> 
> 
>> +struct module_clone_data {
>> +	const char* prefix;
> 
> Asterisk sticks to identifier not type in this codebase, i.e.
> 
> 	const char *prefix;
> 

Will fix and resend.

> Other than that, the split makes sense, especially for the purpose
> of more easily reusing this helper without having to unparse and
> reparse the option strings.
>