Re: [PATCH v7 00/16] Sparse-index: integrate with status

On Wed, Jun 30, 2021 at 7:32 AM Elijah Newren [off-list ref] wrote:

On Mon, Jun 28, 2021 at 7:04 PM Derrick Stolee via GitGitGadget
[off-list ref] wrote:

quoted

This is the first "payoff" series in the sparse-index work. It makes 'git
status' very fast when a sparse-index is enabled on a repository with
cone-mode sparse-checkout (and a small populated set).

...

quoted

Because the range-diff is a big difficult to read this time, I'll break the
changes down on a patch-by-patch basis.

Thanks for doing this; it was helpful.

This is SUPER exciting.  I've only read the cover letter, but it
strongly suggests you've not only handled all my feedback in previous
rounds, but got things pretty solidly nailed away.  I'll try to make
some time to go over it all soon.

You have indeed addressed nearly all my feedback in previous rounds,
and I found few problems with all the new code in this round.
Overall, this round is looking really good, though there are a couple
things I called out in comments on individual patches that I'll
summarize here:

Patch 9: a few minor suggestions for improving comments

Patch 10: the new code is never triggered and probably should either
be dropped or made part of a later series if the later series needs
it.  Also, although it doesn't necessarily need to hold up this
series, I found a bug affecting sparse-checkouts with or without
sparse-index while trying to understand this patch.

Patch 12: the code was slightly confusing to me, but I found that
there seems to be an invariant it is based upon.  Adding an assert
with a comment or just a comment about this invariant might help make
the code more readable.

Patch 15: since the new tests in t1092 are written to compare
sparse-checkout and sparse-index, it seems we should investigate a bug
where the testsuite commands we invoke are giving incorrect behavior
in both sparse-checkout and sparse-index.