Stefan Beller <sbeller@xxxxxxxxxx> writes:
> All submodules which are selected via submodule groups
> ("submodule.groups") are initialized if they were not initialized
> before updating the submodules.
>
> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
> ---
> builtin/submodule--helper.c | 30 +++++++++++++++++++++++++++++-
> t/t7400-submodule-basic.sh | 26 ++++++++++++++++++++++++++
> 2 files changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 4684f16..def382e 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -577,6 +577,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
>
> struct submodule_update_clone {
> /* states */
> + struct string_list *submodule_groups;
Again, do we need a field named submodule_groups in a struct whose
name already makes it clear this is about submodule?
More importantly.
If this were used to implement "there are various groups defined,
and the user tells us that submodules in this and that groups are to
be automatically initialized", then naming the field with a name
that is more specific than just "groups" makes tons of sense, but
even in such a case, the best adjective to clarify what kind of
"group" this field is about is not "this is a submodule group".
The answer I would give to a question "what kind of group this field
is about?" would be "this is an auto-init group", so I'd have that
'auto-init' ness somewhere in its name.
> @@ -633,6 +634,7 @@ static int update_clone_get_next_task(struct child_process *cp,
> const char *update_module = NULL;
> char *url = NULL;
> int needs_cloning = 0;
> + int in_submodule_groups = 0;
Likewise. The significance of the membership in this group is
totally unclear from this variable name. I read this boolean to be
keeping track of "is this submodule to be auto initialized"?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html