From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Fri, 25 Feb 2022 11:42:46 GMT
Subject: Re: [PATCH v3 03/10] t5526: create superproject commits with test helper
Message-ID: <220225.86pmnb8931.gmgdl@evledraar.gmail.com>
In-Reply-To: <kl6lfso74pzm.fsf@chooglen-macbookpro.roam.corp.google.com>


On Fri, Feb 25 2022, Glen Choo wrote:

> Jonathan Tan <jonathantanmy@google.com> writes:
>
>> Glen Choo <chooglen@google.com> writes:
>>> +# For each superproject in the test setup, update its submodule, add the
>>> +# submodule and create a new commit with the submodule change.
>>> +#
>>> +# This requires add_submodule_commits() to be called first, otherwise
>>> +# the submodules will not have changed and cannot be "git add"-ed.
>>> +add_superproject_commits() {
>>> +(
>>> +	cd submodule &&
>>> +	(
>>> +		cd subdir/deepsubmodule &&
>>> +		git fetch &&
>>> +		git checkout -q FETCH_HEAD
>>> +	) &&
>>> +		git add subdir/deepsubmodule &&
>>> +		git commit -m "new deep submodule"
>>> +	) &&
>>
>> The indentation looks off. Also, no need for "-q".
>
> Ah thanks. I think the "-q" is there to suppress the detached HEAD
> warning, which is very large.
>
> I'd prefer to keep it unless there are stronger reasons than "it's not
> needed for correctness". 

FWIW I was going to comment on the -q, but didn't because you're just
moving this around.

I think even for large warnings it's fine to omit -q etc, since that's
what --verbose (as in the test-lib.sh argument) is for.

But in this case it's probably better to leave it as-is.