On Wed, Aug 25, 2021 at 10:19:27AM -0400, Derrick Stolee wrote: > On 8/24/2021 6:37 AM, Patrick Steinhardt wrote: > > Refactor `fetch_refs()` code to make it more extendable by explicitly > > handling error cases. The refactored code should behave the same. [snip] > > - if (!ret) > > - /* > > - * Keep the new pack's ".keep" file around to allow the caller > > - * time to update refs to reference the new objects. > > - */ > > - return 0; > > - transport_unlock_pack(transport); > > - return ret; > > + > > + /* > > + * Keep the new pack's ".keep" file around to allow the caller > > + * time to update refs to reference the new objects. > > + */ > > + return 0; > > And it happens that 'ret' is zero here. Should we keep returning 'ret' > or perhaps add an "assert(!ret);" before the return? The assert() > doesn't do much, but at minimum would serve as an extra indicator to > anyone working in this method in the future. The assert isn't really needed: in the subsequent patch, we always unlock the packfile on exit. Patrick
Attachment:
signature.asc
Description: PGP signature
