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

On Fri, Aug 10, 2018 at 4:33 PM Jeff King [off-list ref] wrote:

On Fri, Aug 10, 2018 at 07:31:33PM -0400, Jeff King wrote:

quoted

On Fri, Aug 10, 2018 at 04:27:25PM -0700, Stefan Beller wrote:

quoted

quoted

 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