Stefan Beller <[email protected]> writes:
> It is also possible to pass in a tree hash to lookup a submodule config.
> Make it clear by naming the variables accrodingly. Looking up a submodule
> config by tree hash will come in handy in a later patch.
>
> Signed-off-by: Stefan Beller <[email protected]>
> ---
Yeah, I noticed that while reading the previous round, too, but...
> -`const struct submodule *submodule_from_name(const unsigned char
> *commit_sha1, const char *name)`::
> +`const struct submodule *submodule_from_name(const unsigned char
> *commit_or_tree, const char *name)`::
>
> The same as above but lookup by name.
Doesn't (the) "above", aka submodule_from_path() want the same
treatment?
Also the explanation of "above" has room for improvement. Namely it
says:
Lookup values for one submodule by its commit_sha1 and path.
I do not think the commit-sha1 (or commit-or-tree-object-name) is
"ITS" object name for the submodule. The name belongs to the
containing superproject commit (or tree), no?
Given a tree-ish in the superproject and a path, return the
submodule that is bound at the path in the named tree.
is what I would write for that one. Thinking about it a bit
further, "treeish_name" would be a much better name for the variable
than "commit_or_tree", as "treeish" is an established short and
sweet word that means "commit_or_tree", and having a "name"
somewhere in the variable name makes it clear that we are not
passing the pointer to an in-core object (e.g. "struct commit *").
> +test_expect_success 'using tree sha1 works' '
> + (
> + cd super &&
> + tree=$(git rev-parse HEAD^{tree}) &&
> + commit=$(git rev-parse HEAD^{commit}) &&
> + test-submodule-config $commit b >expect &&
> + test-submodule-config $tree b >actual &&
> + test_cmp expect actual
> + )
> +'
Perhaps also test a tag that points at the commit?
