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

From: Taylor Blau <me@ttaylorr.com>
Date: Feb 23, 2022, 23:41 UTC
Message-ID: <YhbGIeksK7Df1QzS@nand.local>
In-Reply-To: <xmqqh78pp3k1.fsf@gitster.g>

On Wed, Feb 23, 2022 at 03:26:54PM -0800, Junio C Hamano wrote:

Show 7 quoted lines

> > 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.

Show 12 quoted lines

> >> +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