Re: [PATCH v3 0/8] Maintenance II: prefetch, loose-objects, incremental-repack tasks

From: Derrick Stolee <stolee@gmail.com>
Date: Aug 26, 2020, 16:21 UTC
Message-ID: <9a236846-7631-d999-1579-332d3b3341da@gmail.com>
In-Reply-To: <56C4015E-B431-4B16-8E05-5919BFA71E80@gmail.com>

On 8/26/2020 11:15 AM, Son Luong Ngoc wrote:

Show 14 quoted lines

> Hi Derrick,
> 
>> On Aug 25, 2020, at 20:36, Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote:
>> * Dropped the "verify, and delete and rewrite on failure" logic from the
>>   incremental-repack task. This might be added again later after it can be
>>   tested more thoroughly.
> 
> Perhaps I missed some conversations related to this change but
> why was this verify-rewrite strategy dropped?
> 
> Was the problem such strategy were created to solve is now no longer a concern?
> 
> I feel like it would be much better to add it in and then remove it using a separated commit?
> That way we can follow the reasoning behind these decisions via commit message.

The most-recent message was [1]

[1] https://lore.kernel.org/git/20200819174322.3087791-1-jonathantanmy@google.com/

For now, I'd rather move forward with this simpler task and I will revisit the "verify and fix" situation when it can be done in a focused way instead of being surrounded by builtin boilerplate and other basics of the maintenance feature. Specifically, it would help to have a way to test the logic. In Scalar, I was able to mock the Git commands and return failures in specific places. A similar approach could be done here, or perhaps there is another way to be confident that the "verify and fix" logic is actually helpful.

Thanks, -Stolee