From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 05 Aug 2016 21:16:32 GMT
Subject: Re: [PATCH 5/6] submodule update: add super-reference flag
Message-ID: <xmqq1t22x6fj.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160804195159.7788-6-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---

That's a bit sketchy description.  From the title, I expected that
we would see one additional 'unsigned super_reference : 1;' field in
some structure, but that is not what is happening.  The log message
needs to describe what these string list are and why they are needed
a bit better.

At least, please don't name a multiple_word field "multipleword" ;-)

> @@ -715,6 +716,15 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
>  		for_each_string_list_item(item, &suc->references)
>  			argv_array_pushl(&child->args, "--reference", item->string, NULL);
>  	}
> +	if (suc->superreferences.nr) {
> +		struct string_list_item *item;
> +		for_each_string_list_item(item, &suc->superreferences) {
> +			strbuf_reset(&sb);
> +			argv_array_pushf(&child->args, "--reference=%s/%s",
> +					 relative_path(item->string, suc->prefix, &sb),
> +					 sub->path);

The phrase "super reference" made me imagine it is some kind of
"reference" that is applicable to the superproject, but this code
smells like it is a "prefix to create reference suited for use in
submodule".  Whatever it is, it should be explained better (than "no
desciption" which is what we have here ;-), and given a name that
match the explanation.

Thanks.