On Sat, Nov 4, 2023 at 9:49 AM Jeff King <peff@xxxxxxxx> wrote: > On Sat, Nov 04, 2023 at 02:36:48AM -0400, Eric Sunshine wrote: > > I don't see an urgent need for it. Unlike the actual tests themselves > > run by `make test` which may catch platform-specific problems in Git > > itself, the purpose of the "linting" checks is not to catch > > platform-specific problems, but rather to help test authors by > > identifying mistakes in the tests which might make them fragile. So, > > disabling linting on a particular platform isn't going to cause `make > > test` to miss some important Git problem specific to that platform. > > Hmm. With compilation, we split the audience of "developers" vs "people > who just want to build the program", and we crank up the number and > severity of warning checks for the former. We could do the same here for > tests. I.e., turn off test linting by default and re-enable it for > DEVELOPER=1. My knee-jerk reaction is that this would move us in the wrong direction since it is probable that most drive-by contributors won't have DEVELOPER=1 set, yet they are the ones who are likely to benefit most from test script linting (which is not to say that it doesn't help seasoned contributors, as well). > OTOH, this is the first time I think I've seen the linting cause a > problem (whereas unexpected compile warnings are much more likely, as we > are depending on the system compiler's behavior). There have been a few other times when the linting scripts have needed a tweak or two to work properly on some less-well-represented platform, but they were minor issues[1,2,3,4]. > So consider it an idle thought for discussion, and not necessarily a > proposal. ;) As noted in my response to Junio[5], in the long run, we may want to go with Ævar's idea of having `make` track changes to the test scripts, thus only run linting on an as-needed basis. [1]: a3c4c8841c (tests: use shorter labels in chainlint.sed for AIX sed, 2018-08-24) [2]: 2d9ded8acc (tests: fix comment syntax in chainlint.sed for AIX sed, 2018-08-24) [3]: b3b753b104 (Fit to Plan 9's ANSI/POSIX compatibility layer, 2020-09-10) [4]: 1f51b77f4f (chainlint.pl: fix /proc/cpuinfo regexp, 2022-11-22) [5]: https://lore.kernel.org/git/CAPig+cSC8m5a8PhMw_eJbswwNB-VgBt+n56HSTLLabV9_+y--g@xxxxxxxxxxxxxx/
