On Mon, Oct 19, 2020 at 10:47:35PM +0000, Nipunn Koorapati via GitGitGadget wrote: > test-lint-shell-syntax: > - @'$(PERL_PATH_SQ)' check-non-portable-shell.pl $(T) $(THELPERS) > + @'$(PERL_PATH_SQ)' check-non-portable-shell.pl $(T) $(THELPERS) $(TPERF) I really appreciate your initiative to modify t/Makefile to start linting t/perf/p????-*.sh files, too. Could I bother you to elaborate a little bit on why you chose to modify a recipe in t/Makefile instead of t/perf/Makefile? I'm not necessarily opposed, but having this in t/perf/Makefile would allow me to just run 'make' in 't/perf' and still have the scripts linted there without having to involve a 'make' in 't'. For what it's worth, I suspect that this is because 't/Makefile' already has a 'test-lint-shell-syntax' target, and 't/perf/Makefile' does not. I think it would be OK to add it there, too, and move this change into t/perf. > diff --git a/t/perf/p3400-rebase.sh b/t/perf/p3400-rebase.sh > index d202aaed06..7a0bb29448 100755 > --- 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 && Makes sense. I wouldn't be opposed to breaking this out into an earlier change (e.g., "it's about to become not OK to use seq in t/perf, so prepare for that by replacing any invocations with test_seq()"), but I think it's probably not worth it, since this patch is small as it is. Thanks, Taylor
