Subject: Re: [PATCH 1/4] submodule-config: add submodules_of_tree() helper

From: Junio C Hamano <[email-masked]>
Date: Tue, 23 Nov 2021 14:46:10 -0800

Glen Choo <[email-masked]> writes:

> +struct submodule_entry_list *
> +submodules_of_tree(struct repository *r, const struct object_id *treeish_name)
> +{
> +	struct tree_desc tree;
> +	struct name_entry entry;
> +	struct submodule_entry_list *ret;
> +
> +	CALLOC_ARRAY(ret, 1);
> +	fill_tree_descriptor(r, &tree, treeish_name);
> +	while (tree_entry(&tree, &entry)) {
> +		if (!S_ISGITLINK(entry.mode))
> +			continue;
> +		ALLOC_GROW(ret->name_entries, ret->entry_nr + 1, ret->entry_alloc);
> +		ret->name_entries[ret->entry_nr++] = entry;
> +	}
> +	return ret;
> +}

This only looks at the root level of the tree, doesn't it?  Without
any caller in the same step, it is impossible to tell if that is an
outright bug, or merely an incomplete code that will gain recursion
in a later step.

If it is the latter, I do not think that is a patch series
organization that is very friendly to reviewers.