On Tue, Oct 20, 2020 at 11:17:23PM +0100, Nipunn Koorapati wrote: > > > --- a/t/perf/p3400-rebase.sh > > > +++ b/t/perf/p3400-rebase.sh > > > @@ -9,16 +9,16 @@ test_expect_success 'setup rebasing on top of a lot of changes' ' > > > git checkout -f -B base && > > > git checkout -B to-rebase && > > > git checkout -B upstream && > > > - for i in $(seq 100) > > > + for i in $(test_seq 100) > > > do > > > # simulate huge diffs > > > echo change$i >unrelated-file$i && > > > - seq 1000 >>unrelated-file$i && > > > + test_seq 1000 >>unrelated-file$i && > > > git add unrelated-file$i && > > > test_tick && > > > git commit -m commit$i unrelated-file$i && > > > echo change$i >unrelated-file$i && > > > - seq 1000 | tac >>unrelated-file$i && > > > + test_seq 1000 | tac >>unrelated-file$i && > > > > The rest of this all looks good, but I think adding 'tac' here is still > > wrong; this isn't available everywhere, so we would want to find an > > alternative before going further. Is there a reason that you couldn't > > use a different 'N' in 'test_seq N' here? > > Hey. I think there's some confusion. I didn't add `tac`. It was > already here. I didn't even notice it until Junio mentioned it. You're right, sorry; I just saw a line beginning with '+' that contained 'tac' and thought that it was new in this patch. What you have is OK, then, since it's not a new problem with your patch. It couldn't hurt to have the linting phase catch that, but let's leave that for another day, since I think what you have in this version looks good to me. Thanks for listening to all of my feedback :). > --Nipunn Thanks, Taylor
