Re: [PATCH v3 03/10] t5526: create superproject commits with test helper
- From
Glen Choo <chooglen@google.com>
- Date
- Feb 28, 2022, 18:11 UTC
- Message-ID
- <kl6l8rtuq2sk.fsf@chooglen-macbookpro.roam.corp.google.com>
- In-Reply-To
- <220225.86pmnb8931.gmgdl@evledraar.gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 35 quoted lines
> 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.Ah interesting, I didn't consider that. Thanks!
> But in this case it's probably better to leave it as-is.
I'm also leaning towards this because I'm just moving things around, but I could be convinced otherwise.