Re: [GSoC][PATCH v15 0/9] ref consistency check infra setup

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2489 bytes --]

On Tue, Aug 06, 2024 at 12:43:22AM +0800, shejialuo wrote:
> Hi All:
> 
> This version handles the following problems:
> 
> 1. Patrick advices that I should not use `va_copy` in the changed
> `report` function. Actually this is a mistake, this version avoids
> redundant `ap` copy.
> 2. Patrick advices I should rebase [v14 05/11] into [v14 04/11]. I
> follow this advice in this version.
> 3. Patrick advices that we should put [v14 06/11] before we introduce
> ref-related operations. This version reorders the commit sequence. It's
> a minor change.
> 4. Patrick suggests at current we should not add `git refs verify`
> command into "git-fsck(1)". This is because we should disable this new
> check by default for the users. Many users use "git-fsck(1)" in their daily
> workflow. We should not be aggressive. However, if we provide this
> mechanism in this series, we will again make more complexity. So this
> version drop patch [v14 09/11]. Also because of dropping, change the
> test file to use "git refs verify" command instead of "git fsck"
> command.
> 5. Patrick suggests that we should use `ends_with` instead of
> `strip_suffix`, fix.
> 
> There is another important problem this patch solves:
> 
> At v13, Junio has suggested that the `files_fsck_refs_fn` should be
> adapted to Patrick's change. Actually, I made a bad design before. I
> should always pass the `ref_store` structure. So I change it to
> 
>   -typedef int (*files_fsck_refs_fn)(struct fsck_options *o,
>   -				  const char *gitdir,
>   +typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store,
>   +				  struct fsck_options *o,
>             const char *refs_check_dir,
>             struct dir_iterator *iter);
> 
> `gitdir` could be got by using `ref_store` parameter. By using
> `ref_store` parameter, we provide extensibility here. If something else
> change, we merely need to change "files_fsck_refs_fn" prototype.
> 
> Because I drop one patch and rebase one patch. I provide the `interdiff`
> for reviewers to make the life easier.
> 
> Due to the deadline of the GSoC, I will speed up the review feedback
> process.

I've got another small set of nits, almost not worth addressing. I was a
bit torn whether to send them or not as the series is in a good shape
already, in my opinion. But let's maybe wait one or two more days for
additional feedback, and then (hopefully) reroll this a final time.

Thanks for all your work!

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]