Re: [PATCH v4 11/11] test-lib: split up and deprecate test_create_repo()

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Apr 22, 2021, 12:39 UTC
Message-ID: <87v98e1oj7.fsf@evledraar.gmail.com>
In-Reply-To: <20210421164357.GZ2947267@szeder.dev>

On Wed, Apr 21 2021, SZEDER Gábor wrote:

Show 13 quoted lines

> On Wed, Apr 21, 2021 at 12:15:16PM +0200, Ævar Arnfjörð Bjarmason wrote:
>>  5. Since we don't need to move the .git/hooks directory we don't need
>
> Again: Since we don't need to change directories....
>
>>     the subshell here either.
>> 
>>     See 0d314ce834 for when the subshell use got introduced for the
>>     convenience of not having to "cd" back and forth while setting up
>>     the hooks.
>
> I think this is worse and even more misleading than even v2 was, for
> the reasons I mentioned earlier.

Between this & the thread at http://lore.kernel.org/git/20210420212731.GY2947267@szeder.dev I'm not sure what wording you'd be OK with, would you mind suggesting an alternative?

As far as I can see it's some subtlety of how we explain the cd-ing & subshelling we did as a combination of eedf8f97e58 (Abstract test_create_repo out for use in tests., 2006-02-17) and my 0d314ce834d (test-lib: use subshell instead of cd $new && .. && cd $old, 2010-08-30).

Maybe you'd be happy with this:

    In the code being removed here we both "cd"'d to the repository
    before calling "init", and did so in a subshell. It's not important
    to do either, so both of those can be removed. We cd'd because this
    code grew from test-lib.sh code where we'd have done so already, see
    0d314ce834d, and later "cd"'d inside a subshell since 0d314ce834d to
    avoid having to keep track of an "old pwd" variable to cd back after
    the setup.

    Being in the repository directory made moving the hooks around
    easier (we wouldn't have to fully qualify the path), but since we're
    not moving the hooks anymore we don't need it for that reason
    either.