From: Derrick Stolee <derrickstolee@github.com>
Date: Fri, 16 Dec 2022 13:41:03 GMT
Subject: Re: [PATCH v3 0/4] Optionally skip hashing index on write
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:
> 
> 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.


> 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.
 
> 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