Re: [PATCH v4] fetch: allow adding a filter after initial clone.

From: Xin Li <delphij@google.com>
Date: May 29, 2020, 06:54 UTC
Message-ID: <CAOhzduh4V4LKgzksTkSdEDrgsv59jTpfYDBTUzZvRxZOaqhhmg@mail.gmail.com>
In-Reply-To: <20200529010119.GE114915@google.com>

Regarding t/t2404-worktree-config.sh:

On Thu, May 28, 2020 at 6:01 PM Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 15 quoted lines

> Xin Li wrote:
>
> > diff --git a/t/t2404-worktree-config.sh b/t/t2404-worktree-config.sh
> > index 286121d8de..9536d10919 100755
> > --- a/t/t2404-worktree-config.sh
> > +++ b/t/t2404-worktree-config.sh
> > @@ -23,8 +23,10 @@ test_expect_success 'config --worktree without extension' '
> >  '
> >
> >  test_expect_success 'enable worktreeConfig extension' '
> > +     git config core.repositoryformatversion 1 &&
> >       git config extensions.worktreeConfig true &&
>
> Yes, makes sense.  Does this patch need it, or could this go in a
> separate patch?

Yes, this patch needs setting repositoryformatversion to 1 as we would no longer recognize extensions.worktreeConfig=true on version 0 repositories.

Show 6 quoted lines

> > -     test_cmp_config true extensions.worktreeConfig
> > +     test_cmp_config true extensions.worktreeConfig &&
> > +     test_cmp_config 1 core.repositoryformatversion
>
> This (both the existing code and the modified version) is strange: we
> just set the config, so why are we checking it?

The check was mainly to match the existing pattern (which sets extensions.worktreeConfig and immediately asserts that they were set). These assertions are not strictly necessary but are harmless, so I don't feel strongly about keeping or removing them.