On 03/23, Duy Nguyen wrote:
> On Fri, Mar 23, 2018 at 6:03 PM, Duy Nguyen <pclo...@gmail.com> wrote:
> > On Wed, Mar 21, 2018 at 11:18 PM, Brandon Williams <bmw...@google.com> 
> > 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

Reply via email to