On 03/23, Duy Nguyen wrote: > On Fri, Mar 23, 2018 at 6:03 PM, Duy Nguyen <pclouds@xxxxxxxxx> wrote: > > On Wed, Mar 21, 2018 at 11:18 PM, Brandon Williams <bmwill@xxxxxxxxxx> wrote: > >> You're marking packed_git > >> as "private"...well C has no notion of private vs public fields in a > >> struct so it might be difficult to keep that convention, it also took me > >> a second to realize that it was only in the scope of packfile.c where it > >> was ok to reference it directly. Maybe it'll be ok? If we really > >> wanted something to be private we'd need it to be an opaque type > >> instead, which may be out of the scope of this code refactor. > > > > It's true C syntax does not support private/public scoping, but it > > does not mean we must avoid that. Python has "private convention" with > > the underscore prefix, no special support from the language either. > > Yes having compiler support to enforce scoping is nice, but I think we > > can still live without it (Go devs live fine without "const" > > arguments, e.g.) > > > > So I'm going to make it clearer that these fields are not supposed to > > be accessed outside packfile.c > > I'm not counting out the making these fields completely opaque of > course. And with your suggestion of not embedding raw_object_store to > repository, that's actually possible to do. But I'm still not doing it > now :) The series is getting long and extending its scope will drag it > even longer (in terms of both time and number of patches) Oh of course. I'm a big proponent of taking small steps, so definitely avoid scope creep and hopefully this series can land quicker so we have a better base to work with to make some of those other improvements if we still want to down the road. -- Brandon Williams