Re: [PATCH 2/2] t5302: confirm that large packs mention limit

From: Taylor Blau <me@ttaylorr.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Taylor Blau <me@ttaylorr.com>,
	Matt Cooper via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org, derrickstolee@github.com,
	Matt Cooper <vtbassmatt@gmail.com>
Subject: Re: [PATCH 2/2] t5302: confirm that large packs mention limit
Date: Wed, 23 Feb 2022 18:41:21 -0500	[thread overview]
Message-ID: <YhbGIeksK7Df1QzS@nand.local> (raw)
In-Reply-To: <xmqqh78pp3k1.fsf@gitster.g>

On Wed, Feb 23, 2022 at 03:26:54PM -0800, Junio C Hamano wrote:
> > It's not a huge deal, and I'm sure we have plenty of examples of
> > slightly out-of-order commit trailers throughout our history. Personally
> > I don't consider it worth rerolling, but perhaps something to keep in
> > mind for future contributions :-).
>
> People need to understand that each such contributor robs maintainer
> bandwidth by not rerolling.

I figured you weren't going to tweak the trailers when applying, hence
"not a huge deal" in my comment above. But if it's going to cause you to
spend extra effort in order to pick these patches up, then I'm sure Matt
would be happy to reroll.

> >> +test_expect_success 'too-large packs report the breach' '
> >> +	pack=$(git pack-objects --all pack </dev/null) &&
> >> +	sz="$(test_file_size pack-$pack.pack)" &&
> >> +	test "$sz" -gt 20 &&
> >> +	test_must_fail git index-pack --max-input-size=20 pack-$pack.pack 2>err &&
> >> +	grep "maximum allowed size (20 bytes)" err
> >> +'
>
> This test looks OK to me.
>
> Shouldn't it be squashed into the previous patch?  After all, it is
> a test for the new behaviour introduced by the previous step, right?

Yeah. I mentioned in my response to Matt's cover letter that I figured
the two could be squashed together, and that there was no reason to keep
them separate.

(That was written under the assumption that he wasn't going to send a
reroll, in which case the advice was along the lines of "not a big deal
for this instance, but in the future...")

Thanks,
Taylor