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

From: Derrick Stolee <stolee@gmail.com>
Date: Mar 17, 2021, 20:27 UTC
Message-ID: <9424d5c1-68d4-f6dc-7fb1-3b5617d3ad48@gmail.com>
In-Reply-To: <87tup9zybl.fsf@evledraar.gmail.com>

On 3/17/2021 9:47 AM, Ævar Arnfjörð Bjarmason wrote:

Show 19 quoted lines

> 
> On Tue, Mar 16 2021, Derrick Stolee wrote:
> 
>> On 3/16/2021 12:17 PM, Ævar Arnfjörð Bjarmason wrote:
>>> 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?

Yeah, if it is frequently paired with fsck operations, I think it makes the most sense there.

And looking at it again, I'm not sure parse-options-cb.c has a good excuse for being separate from parse-options.c, but that's the current state so I wouldn't change it now.

Thanks, -Stolee