Re: [PATCH v2 0/8] rev-parse: implement object type filter

[Jeff King <peff@peff.net>]

On Sat, Mar 20, 2021 at 02:10:41PM -0700, Junio C Hamano wrote:

> Patrick Steinhardt <ps@pks.im> writes:
> 
> > this is the second version of my patch series which implements a new
> > `object:type` filter for git-rev-parse(1) and git-upload-pack(1) and
> > extends support for bitmap indices to work with combined filters.
> > ...
> > Please see the attached range-diff for more details.
> 
> Any comment from stakeholders?

Sorry, this languished on my to-review list for a while.

I took a careful look. I found a few small nits, but the code overall
looks pretty good.

I do still find the use of the filter code here a _little_ bit
off-putting. It makes perfect sense in some ways: we are asking rev-list
to filter the output, and it keeps our implementation nice and simple.
It took me a while to figure out what I think makes it weird, but I
think it's:

  - the partial-clone feature exposes the filter mechanism in a very
    transparent way. So while it's not _wrong_ to be able to ask for a
    partial clone of only trees, it's an odd thing that nobody would
    really use in practice. And so it's a bit funny that it gets
    documented alongside blob:limit, etc.

  - for the same reason, it's very rigid. We have no way to say "this
    filter OR that filter", and are unlikely to grow them (because this
    is all part of the network protocol). Whereas it's perfectly
    reasonable for somebody to ask for "trees and blobs" via rev-list.

I dunno. Those aren't objections exactly. Just trying to put my finger
on why my initial reaction was "huh, why --filter?".

-Peff