Re: [PATCH v2 0/9] Sparse index: integrate with 'clean', 'checkout-index', 'update-index'

From: Derrick Stolee <stolee@gmail.com>
To: Elijah Newren <newren@gmail.com>,
	Victoria Dye via GitGitGadget <gitgitgadget@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>,
	Victoria Dye <vdye@github.com>
Subject: Re: [PATCH v2 0/9] Sparse index: integrate with 'clean', 'checkout-index', 'update-index'
Date: Thu, 27 Jan 2022 11:36:03 -0500	[thread overview]
Message-ID: <771fa224-4319-c827-d28d-51e767b0050e@gmail.com> (raw)
In-Reply-To: <CABPp-BEVM-nhTX+DRf3NN16G-5Wr2KzObCiETqcy+ZYJLR4EWQ@mail.gmail.com>

n 1/12/2022 10:02 PM, Elijah Newren wrote:
> On Tue, Jan 11, 2022 at 10:05 AM Victoria Dye via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>>
>> This series continues the work to integrate commands with the sparse index,
>> adding integrations with 'git clean', 'git checkout-index', and 'git
>> update-index'. These three commands, while useful in their own right, are
>> updated mainly because they're used in 'git stash'. A future series will
>> integrate sparse index with 'stash' directly, but its subcommands must be
>> integrated to avoid the performance cost of each one expanding and
>> collapsing the index.
>>
>> The series is broken up into 4 parts:
>>
>>  * Patches 1-2 are minor fixups to the 'git reset' sparse index integration
>>    in response to discussion [1] that came after the series was ready for
>>    merge to 'next'.
>>  * Patch 3 integrates 'git clean' with the sparse index.
>>  * Patches 4-6 integrate 'git checkout-index' with the sparse index and
>>    introduce a new '--ignore-skip-worktree-bits' option.
>>    * This involves changing the behavior of 'checkout-index' to respect
>>      'skip-worktree' by default (i.e., it won't check out 'skip-worktree'
>>      files). The '--ignore-skip-worktree-bits' option can be specified to
>>      force checkout of 'skip-worktree' files, if desired.
>>  * Patches 7-9 integrate 'git update-index' with the sparse index.
>>    * Note that, although this integrates the sparse index with
>>      '--cacheinfo', sparse directories still cannot be updated using that
>>      option (see the prior discussion [2] for more details on why)
>>
>>
>> Changes since V1
>> ================
>>
>>  * Changed 'checkout-index' to fail by default when given filenames of files
>>    with 'skip-worktree' enabled
>>    * These files can still be forcibly checked-out by using the
>>      '--ignore-skip-worktree-bits' option
>>    * Added/updated corresponding t1092 tests
>>  * Updated t1092 'update-index' tests
>>    * Mentioned where/why 'skip-worktree' files were manually created on-disk
>>      for testing purposes
>>    * Provided explanation as to what '--remove' does, and how it relates to
>>      '--ignore-skip-worktree-entries'; restructured corresponding test
>>    * Fixed typo 'update-indexe' -> 'update-index'
>>    * Removed unused 'edit-contents'
>>    * Changed '--again' test to not use '--remove' to avoid confusion over
>>      how/why it updates 'skip-worktree' entries
>>    * Added "set skip-worktree" step to '--cacheinfo' test to illustrate how
>>      it could be used to add a new outside-of-cone file and remain generally
>>      compliant with a sparse-checkout definition
>>    * Added '--cacheinfo' test to "ensure not expanded"
>>    * Moved t1092 test 'sparse index is not expanded: update-index' to avoid
>>      merge conflict
>>  * Updated p2000 test for 'update-index': added file argument
>>    * Without any file arguments, 'update-index' was effectively a no-op
>>  * Clarified reasoning behind changing/not changing behavior of update-index
>>    in sparse-checkouts
> 
> Nicely done!  You've addressed all my (voluminous) feedback from v1;
> this round looks good to me.
> 
> Reviewed-by: Elijah Newren <newren@gmail.com>

Thank you for that review, Elijah. I took this opportunity to
reread the series as well as the range-diff and I agree that
this version is ready to go.

Thanks,
-Stolee