On 15-Jun-2021, at 09:21, Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Atharva Raykar <raykar.ath@xxxxxxxxx> 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. >