From: Jiang Xin <worldhello.net@gmail.com>
Date: Mon, 28 Nov 2022 09:43:49 GMT
Subject: Re: [PATCH v1 3/4] t1301: wrap the statements in the for loop
Message-ID: <CANYiYbGwHQiDh7qf-LTtuUM7eC3AEpkuXoejOBkJs-VGZTA14A@mail.gmail.com>
In-Reply-To: <xmqq35a390ek.fsf@gitster.g>

On Mon, Nov 28, 2022 at 12:19 PM Junio C Hamano <gitster@pobox.com> wrote:
> Now you have three separate tests in an interation of the loop.  If
> you skipped the first one in the iteration (by mistake) and let the
> other two run, they will run with a wrong configuration and values
> of $x and $y variables, either unset or leftover from the previous
> round.
>
> So I am not sure how this patch can be an improvement.

I agree that this patch is not that necessary as the other 3 patches
and will remove it in next reroll.

>
> If you wrapped the setting of $x, $y, $u and the config into a
> helper shell function, e.g.
>
>         prepare_perm_test_variables () {
>                 u=$1
>                 x=...
>                 y=...
>                 u=...
>                 git config core.sharedrepository "$u"
>         }
>
> before and outside the loop, and make these two tests in the loop to
> call it upfront, then your users can skip each test and iteration
> independently while ensuring that the necessary setup is always done
> correctly, though.
>