From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 25 Feb 2022 00:33:41 GMT
Subject: Re: [PATCH v3 09/10] fetch: fetch unpopulated, changed submodules
Message-ID: <xmqqsfs7hjiy.fsf@gitster.g>
In-Reply-To: <20220224100842.95827-10-chooglen@google.com>

Glen Choo <chooglen@google.com> writes:


> +static struct fetch_task *
> +get_fetch_task_from_changed(struct submodule_parallel_fetch *spf,
> +			    struct strbuf *err)
> +{
> +	for (; spf->changed_count < spf->changed_submodule_names.nr;
> +	     spf->changed_count++) {
> +		struct string_list_item item =
> +			spf->changed_submodule_names.items[spf->changed_count];
> +		struct changed_submodule_data *cs_data = item.util;
> +		struct fetch_task *task;
> +
> +		if (!is_tree_submodule_active(spf->r, cs_data->super_oid,cs_data->path))
> +			continue;

Where does this function come from?  I seem to be getting compilation errors.

> diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
> index ee4dd5a4a9..639290d30d 100755
> --- a/t/t5526-fetch-submodules.sh
> +++ b/t/t5526-fetch-submodules.sh
> @@ -15,8 +15,9 @@ pwd=$(pwd)
>  
>  check_sub() {

Style.  

	check_sub () {

>  	NEW_HEAD=$1 &&
> +	SUPER_HEAD=$2 &&
>  	cat <<-EOF >$pwd/expect.err.sub

Style.

	cat <<-EOF >"$pwd/expect.err.sub"

You may swap the order of redirection (having <<here-doc at the end
of the line might look more familiar to some people).  Try to do as
majority of surrounding code does.

Make sure you quote the redirection target filename if it involves
variable interpolation (see Documentation/CodingGuidelines, look for
"Redirection").

> +	cat <<-EOF > expect.err.combined &&

Style.

	cat <<-EOF >expect.err.combined &&

No SP between redirection operator and its target.

> +	sed -E "s/[0-9a-f]+\.\./OLD_HEAD\.\./" actual.err >actual.err.cmp &&

No ERE in sed.  "[0-9a-f][0-9a-f]*" instead of "[0-9a-f]+" should be
sufficient, I think.