Re: [PATCH 1/7] for_each_*_object: store flag definitions in a single location

[Stefan Beller <sbeller@google.com>]

On Fri, Aug 10, 2018 at 4:33 PM Jeff King <peff@peff.net> wrote:
>
> On Fri, Aug 10, 2018 at 07:31:33PM -0400, Jeff King wrote:
>
> > On Fri, Aug 10, 2018 at 04:27:25PM -0700, Stefan Beller wrote:
> >
> > > >  cache.h    | 13 ++++++++++++-
> > > >  packfile.h |  8 ++------
> > > >  2 files changed, 14 insertions(+), 7 deletions(-)
> > >
> > > rubs me the wrong way. ;-)
> > >
> > > cache.h is such a misnomer of a name, and a kitchen sink
> > > of a file in the Git project that in an ideal world it would
> > > be way smaller and contain only things related to some
> > > caching related code.
> > >
> > > I would suggest object.h or object-store.h instead.
> > > Probably the object-store as that will be the only external
> > > exposure and hopefully we'd get the objects in a similar
> > > shape as the refs subsystem eventually?
> >
> > Yes, for_each_loose_object() ought to be in loose.h to match packfile.h,
> > or the whole thing should go into object-store.h.
>
> Heh, I thought you were making up a hypothetical object-store.h, but I
> see it has already come to pass.
>
> IMHO the whole for_each_*_object() interface should go in there (it even
> has packed_git defined there already!). I think I'd still just as soon
> do it on top of this series, but it might not be too bad to do as part
> of a re-roll.

Yeah, I realize that I distracted myself and ranted about a different thing
other than the quality of this patch. (We had a couple of internal discussions
about project velocity and contributor happiness and I personally think this
derailing is some sort of anti pattern as fixing things like these is easy
as compared to user visible things such as file formats or configs.
Sorry for that.)

Stefan