Re: What's cooking in git.git (Sep 2025, #06; Mon, 15)

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 16, 2025, 16:16 UTC
Message-ID: <xmqqzfaunzjo.fsf@gitster.g>
In-Reply-To: <aMkJVMbSmeA4cIAy@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 14 quoted lines

> On Mon, Sep 15, 2025 at 12:19:45PM -0700, Junio C Hamano wrote:
>> * ps/odb-clean-stale-wrappers (2025-09-12) 2 commits
>>  - fixup! odb: drop deprecated wrapper functions
>>  - odb: drop deprecated wrapper functions
>> 
>>  Code clean-up.
>> 
>>  Breaks build when merged to 'seen'.
>>  cf. <20250910153759.GA562601@coredump.intra.peff.net>
>>  source: <20250910-b4-pks-odb-drop-wrappers-v1-1-6ed660cb1eec@pks.im>
>
> Shall I send a new revision of this patch series that squashes in the
> fixup commit? I wouldn't mind doing that, but it becomes a bit weird to
> do so when the original base of the patch doesn't even the issue.

Yeah, typicall fallouts from code churn that needs to be handled somewhere in the codebase. *hit happens (shrug).

As this makes it necessary for that file to eventually include an extra header, it sort of makes sense. I am more wondering if it should be a separate commit (i.e. give it a proper log message instead of fixup!). If we were to squash, we would need to mention why a seemingly unnecessary change is included. "In anticipation of another topic that adds a call to function Y, whose definition this topic shuffles around and makes it necessary for its callers to include header X, we pre-emptively include header X that will become needed for the other topic to use function Y when merged with this topic". I agree that is certainly awkward.

Adding the extra include to the other topic is not any cleaner. It didn't have to include that header to make calls to some functions, and it is only because another topic shuffled things around that made it necessary. "In anticipation of another topic shuffling headers around, we pre-emptively include header X that will become needed to use function Y when merged with the other topic" would be such an extra commit would say. That may be slightly less awkward but it still is so.

And it would be cumbersome to do _the_ right thing for something so small like this: build this on top of the topics affected. That would allow us to say something like "We will move things around and require header X to be included by callers of function Y in a later commit in this series. Do so now as a preliminary step."

So I dunno.  What's your preference?