On Tue, Jan 23, 2024 at 01:50:04PM -0800, Junio C Hamano wrote: > Junio C Hamano <gitster@xxxxxxxxx> writes: > > > A comment and a half. > > > > * Can't the new "how to flush" go to the write-option structure? > > If you represent "no flush" as a NULL pointer in the flush member, > > most of the changes to the _test files can go, no? > > Nah, that was a stupid comment. These are used to populate the > members of the reftable_writer instance being created, and it does > make sense to have flush_func immediately next to writer_func. Agreed (not on the "stupid" part, on having it next to `writer_func`). > The part about using NULL as the value to say "do not use any flusher" > still stands, though. You do not have to expose noop_flush into the > global namespace that way. One benefit of explicitly using the `noop_flush()` function is that we make sure that all callsites that should provide a proper flushing function indeed do. A `noop_flush` in production code may raise some eyebrows, whereas a `NULL` value could easily be overlooked. Whether that is a good enough reason for the additional churn might be a different question. I don't think it's particularly bad though. Patrick
Attachment:
signature.asc
Description: PGP signature
