Taylor Blau <me@xxxxxxxxxxxx> writes: > On Wed, Feb 23, 2022 at 04:03:13PM +0000, Matt Cooper via GitGitGadget wrote: >> From: Matt Cooper <vtbassmatt@xxxxxxxxx> >> >> When a pack can't be processed because it's too large, we report the >> exact nature of the breach. This test ensures that the error message has >> a human-readable size included. >> >> Signed-off-by: Matt Cooper <vtbassmatt@xxxxxxxxx> >> Helped-by: Taylor Blau <me@xxxxxxxxxxxx> >> Helped-by: Derrick Stolee <derrickstolee@xxxxxxxxxx> > > Ah, one small note here: typically we try to keep commit trailers in > reverse-chronological order, with the most recent thing at the bottom. > That doesn't really matter for the Helped-by's, but keeping your s-o-b > at the bottom indicates that you signed off on the result of your patch > after Stolee and I gave some suggestions. It is very much appreciated to point these things out. > 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. >> +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? Thanks.
