Re: [PATCH v5 01/14] bulk-checkin: rename 'state' variable and separate 'plugged' boolean

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux