On Tue, Oct 20, 2020 at 3:39 AM Taylor Blau <me@xxxxxxxxxxxx> wrote: > > 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. Looked at doing this and noticed that there are several targets in test-lint in t/Makefile. This would involve duplicating them into t/perf/Makefile which seems like it would be poor form, especially given their complexity. Perhaps t/perf/Makefile could have a target which calls t/Makefile's test-lint target instead. Will play around with it. > > 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. > Yeah - I see the point, but I agree that since the patch is small, it's ok this way. If the patch grows significantly, I can make it into two patches --Nipunn