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

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

Hi Derrick,

quoted

On Aug 25, 2020, at 20:36, Derrick Stolee via GitGitGadget [off-list ref] 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/ (local)

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