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

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> 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?