Re: [PATCH v2 08/23] pack-bitmap: make `bitmap_writer_push_bitmapped_commit()` public

From: Taylor Blau <me@ttaylorr.com>
Date: May 14, 2024, 00:54 UTC
Message-ID: <ZkK2Y06dgyLanNEA@nand.local>
In-Reply-To: <20240513185055.GB2675586@coredump.intra.peff.net>

On Mon, May 13, 2024 at 02:50:55PM -0400, Jeff King wrote:

Show 18 quoted lines

> On Mon, Apr 29, 2024 at 04:43:22PM -0400, Taylor Blau wrote:
>
> > The pseudo-merge selection code will be added in a subsequent commit,
> > and will need a way to push the allocated commit structures into the
> > bitmap writer from a separate compilation unit.
> >
> > Make the `bitmap_writer_push_bitmapped_commit()` function part of the
> > pack-bitmap.h header in order to make this possible.
>
> I was a little surprised that this function and the one in the previous
> commit needed to be public, since this whole topic is restricted to
> writing, which is mostly contained to pack-bitmap-write.c. But you've
> pulled the pseudo-merge bits out to pseudo-merge.[ch], and they need
> access, which makes sense.
>
> One could argue that it could all get stuffed into pack-bitmap-write.c,
> but that is already getting to be a pretty large and complex file. So
> this is probably the best route.

I had originally written the series like that, but the new bits nearly doubled the line count of pack-bitmap-write.c:

    $ wc -l pack-bitmap-write.c pseudo-merge.c
     1031 pack-bitmap-write.c
      752 pseudo-merge.c
     1783 total

so I ended up splitting it out into pseudo-merge.ch in the end.

Thanks, Taylor