From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 23 Nov 2021 22:46:10 GMT
Subject: Re: [PATCH 1/4] submodule-config: add submodules_of_tree() helper
Message-ID: <xmqqh7c27aod.fsf@gitster.g>
In-Reply-To: <20211122223252.19922-2-chooglen@google.com>

Glen Choo <chooglen@google.com> 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.