Subject: Re: [PATCH v4 22/22] fetch-pack: use new fsck API to printing dangling submodules

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Wed, 17 Mar 2021 14:47:42 +0100

On Tue, Mar 16 2021, Derrick Stolee wrote:

> On 3/16/2021 12:17 PM, Ævar Arnfjörð Bjarmason wrote:
>> Refactor the check added in 5476e1efde (fetch-pack: print and use
>> dangling .gitmodules, 2021-02-22) to make use of us now passing the
>> "msg_id" to the user defined "error_func". We can now compare against
>> the FSCK_MSG_GITMODULES_MISSING instead of parsing the generated
>> message.
>> 
>> Let's also replace register_found_gitmodules() with directly
>> manipulating the "gitmodules_found" member. A recent commit moved it
>> into "fsck_options" so we could do this here.
>> 
>> Add a fsck-cb.c file similar to parse-options-cb.c, the alternative
>> would be to either define this directly in fsck.c as a public API, or
>> to create some library shared by fetch-pack.c ad builtin/index-pack.
>> 
>> I expect that there won't be many of these fsck utility functions in
>> the future, so just having a single fsck-cb.c makes sense.
>
> I'm not convinced that having a single cb function merits its
> own file. But, if you expect this pattern to be expanded a
> couple more times, then I would say it is worth it. Do you have
> such plans?

Not really, well. Vague ones, but nothing I have even local patches for.

It just seemed odd to stick random callback functions shared by related
programs into fsck.h's interface, but I guess with
FSCK_OPTIONS_MISSING_GITMODULES I already did that.

Do you suggest just putting it into fsck.c?