From: Glen Choo Date: Mon, 28 Feb 2022 18:11:39 GMT Subject: Re: [PATCH v3 03/10] t5526: create superproject commits with test helper Message-ID: In-Reply-To: <220225.86pmnb8931.gmgdl@evledraar.gmail.com> Ævar Arnfjörð Bjarmason writes: > On Fri, Feb 25 2022, Glen Choo wrote: > >> Jonathan Tan writes: >> >>> Glen Choo 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.