Re: [PATCH v3 0/4] Optionally skip hashing index on write

From: Derrick Stolee <derrickstolee@github.com>
Date: Dec 16, 2022, 13:41 UTC
Message-ID: <9e754af8-ecd3-6aed-60e8-2fc09a6a8759@github.com>
In-Reply-To: <221215.865yec3b1j.gmgdl@evledraar.gmail.com>

On 12/15/22 10:56 AM, Ævar Arnfjörð Bjarmason wrote:

Show 9 quoted lines

> 
> On Thu, Dec 15 2022, Derrick Stolee via GitGitGadget wrote:
> 
>> Updates since v2
>> ================
>>
>>  * Patch 2 now has additional details about why to use the config option in
>>    its message. The discussion about why the index is special is included in
>>    the commit message.

Show 8 quoted lines

> So even if we can't produce a behavior difference, just doing e.g.:
> 
> 	struct repository *r = istate->repo ? istate->repo : the_repository;
> 
> And then using:
> 
> 	prepare_repo_settings(r);
>         f->skip_hash = r->->settings.index_skip_hash;

This, and other comments are sensible and will be reflected in v4.

Show 5 quoted lines

> Seems sensible. I just don't get why 4/4 has that seeming fix-up of 2/4
> after-the-fact. Isn't it better to carve that bit out of 4/4, just do
> the config setup in repo-settings.c to begin with, and have 4/4 do what
> its $subject says, i.e. to have "feature.manyFiles" imply this new
> config?

The point is to make patch 4 completely independent, and be able to pull it out if necessary. There's no reason to load the config inside repo-settings.c unless it's part of something like feature.manyFiles.

Thanks, -Stolee