On Sat, Aug 14, 2021 at 11:05:41AM -0700, Junio C Hamano wrote:
>
> "Mahi Kolla via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>
> > diff --git a/builtin/clone.c b/builtin/clone.c
> > index 66fe66679c8..a08d9012243 100644
> > --- a/builtin/clone.c
> > +++ b/builtin/clone.c
> > @@ -986,6 +986,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> > struct remote *remote;
> > int err = 0, complete_refs_before_fetch = 1;
> > int submodule_progress;
> > + int sticky_recursive_clone;
>
> This variable does not have to be in such a wider scope, I think.
>
>
> > struct transport_ls_refs_options transport_ls_refs_options =
> > TRANSPORT_LS_REFS_OPTIONS_INIT;
> > @@ -1130,6 +1131,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> > strbuf_detach(&sb, NULL));
> > }
>
> Just in this scope, where "struct string_list_item *item" and
> "struct strbuf sb" are declared, is where an extra int variable,
> which receives the configuration value, needs to exist.
>
> Also, for a variable that is used only to receive value from
> git_config_get_bool(), immediately to be used and then never used
> again, we do not need such a long and descriptive name.
>
> > + if (!git_config_get_bool("submodule.stickyRecursiveClone", &sticky_recursive_clone)
> > + && sticky_recursive_clone) {
> > + string_list_append(&option_config, "submodule.recurse=true");
> > + }
>
> We do not need {} around a single statement block.
>
> Taken together, perhaps like the attached.
>
> I'll queue the patch posted as-is for now.
>
> Thanks.
>
> diff --git c/builtin/clone.c w/builtin/clone.c
> index 66fe66679c..c4e02d2f78 100644
> --- c/builtin/clone.c
> +++ w/builtin/clone.c
> @@ -1114,6 +1114,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> if (option_recurse_submodules.nr > 0) {
> struct string_list_item *item;
> struct strbuf sb = STRBUF_INIT;
> + int val;
>
> /* remove duplicates */
> string_list_sort(&option_recurse_submodules);
> @@ -1130,6 +1131,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> strbuf_detach(&sb, NULL));
> }
>
> + if (!git_config_get_bool("submodule.stickyRecursiveClone", &val) &&
> + val)
> + string_list_append(&option_config, "submodule.recurse=true");
> +
> if (option_required_reference.nr &&
> option_optional_reference.nr)
> die(_("clone --recursive is not compatible with "
I like the changes you propose here. It also looks to me like you wanted
to wait for Mahi to send the updates herself, which I appreciate.
Mahi's internship ended last week and I believe she told me she's
planning to be very busy this week; so if we don't hear from her by this
time next week, I'll send a reroll with these changes (unless you want
to just take them yourself without the list churn).
- Emily
