Re: [PATCH v4 5/8] list-objects: implement object type filter

From: Patrick Steinhardt <ps@pks.im>
Date: Apr 14, 2021, 11:32 UTC
Message-ID: <YHbSxl3cVsJUSKGp@ncase>
In-Reply-To: <877dl6eccv.fsf@evledraar.gmail.com>

On Tue, Apr 13, 2021 at 11:57:04AM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 11 quoted lines

> 
> On Mon, Apr 12 2021, Patrick Steinhardt wrote:
> 
> > +	} else if (skip_prefix(arg, "object:type=", &v0)) {
> > +		int type = type_from_string_gently(v0, -1, 1);
> > +		if (type < 0) {
> > +			strbuf_addstr(errbuf, _("expected 'object:type=<type>'"));
> 
> Maybe:
> 
> 	strbuf_addstr(errbuf, _("'%s' for 'object:type=<type>' is not a valid object type"), v0);

`strbuf_addf`, but otherwise I agree that it does make for a more actionable error message.

Show 18 quoted lines

> > +# Test object:type=<type> filter.
> > +
> > +test_expect_success 'setup object-type' '
> > +	git init object-type &&
> > +	echo contents >object-type/blob &&
> > +	git -C object-type add blob &&
> > +	git -C object-type commit -m commit-message &&
> > +	git -C object-type tag tag -m tag-message
> > +'
> 
> Does this really need to not be the shorter:
> 
>     test_create_repo object-type &&
>     test_commit -C object-type blob
> 
> Or if it really needs the annotated tag maybe that + --no-tag + create
> the tag after, and it can eventually use my
> https://lore.kernel.org/git/patch-06.16-8d43fdd5865-20210412T110456Z-avarab@gmail.com/

It must be an annotated tag because that's what the `--object` flag of git-rev-list(1) is working on: objects. But anyway, as you say I can still use these two helpers and then manually create the tag.

Show 8 quoted lines

> > +test_expect_success 'verify object:type=blob prints blob and commit' '
> > +	(
> > +		git -C object-type rev-parse HEAD &&
> > +		printf "%s blob\n" $(git -C object-type rev-parse HEAD:blob)
> > +	) >expected &&
> 
> Just use > and >> or a single printf with two arguments instead of a
> subshell?

I don't really mind, happy to take this proposal.

Thanks!

Patrick