On Wed, Mar 30, 2022 at 1:24 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Neeraj Singh <nksingh85@xxxxxxxxx> writes: > > >> We should add a new function, flush_bulk_checking_packfile(), to > >> flush only the packfile part of the bulk_checkin_state without > >> affecting other things---the "plugged" bit is the only one in the > >> current code before this series, but it does not have to stay to be > >> so > > > > I'm happy to rename the packfile related stuff to end with _packfile > > to make it clear that all of that state and functionality is related > > to batching of packfile additions. > > I do not care about names, though. If you took that I hinted any > such change, sorry about that. _state is fine as-is. > > I do care about not ejecting plugged out of the structure and > instead keeping them together, with proper way to flush the part > that deflate_to_pack() wants to flush, instead of abusing the > "finish". > > Thanks. Just to understand your feedback better, is it a problem to separate the state of each separate "thing" under ODB transactions into separate file-scope global(s)? In this series I declared the fsync state as completely separate from the packfile state. That's why I was thinking of it as more of a naming problem, since the remaining state aside from the plugged boolean is entirely packfile related. My argument in favor of having separate file-scoped variables for each 'pluggable thing' would be that future implementations can evolve separately without authors first having to disentangle a single struct. Thanks, Neeraj