Re: [PATCH 3/3] gc: perform incremental repack when implictly enabled

From: Glen Choo <chooglen@google.com>
Date: Sep 14, 2021, 17:41 UTC
Message-ID: <YUDe1Spvh4r7VyA5@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <YT+odsA17BqtD+WJ@nand.local>

On Mon, Sep 13, 2021 at 03:37:26PM -0400, Taylor Blau wrote:

Show 8 quoted lines

> > builtin/gc.c has two ways of checking of multi-pack-index is enabled:
> > - git_config_get_bool() in incremental_repack_auto_condition()
> > - the_repository->settings.core_multi_pack_index in
> >   maintenance_task_incremental_repack()
> 
> Is this hinting at why there are no new tests added here? If so, it may
> need to be explained more clearly, since I was a tad confused after
> reading it.

Looks like I'll need to be clearer; this wasn't my intention at all. I was hoping to describe the current state of affairs and to show that there are two different approaches. Thus, this patch is an attempt to 'standardize' the two approaches.

Show 5 quoted lines

> But if not, this patch message deserves an extra sentence or
> two saying why tests aren't necessary.
>
> Or if none of that is the case, and tests *are* necessary, then they
> should be added ;).

I initially did not include tests because it *seemed* to me that there were no tests for this. But on a second pass, it looks like that assumption was completely wrong (the tests are in t7900-maintenance.sh).

So, tests are necessary, and so I will add them :)