On Wed, Jul 11, 2018 at 5:37 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
> As with the previous "transform the script and feed the result to
> shell" approach, this risks to force us into writing our tests in a
> subset of valid shell language, which is the primary reason why I
> was not enthused when I saw the previous round. The worst part of
> it is that the subset is not strictly defined based on the shell
> language syntax or features (e.g. we allow this and that feature but
> not that other feature) but "whatever that does not cause the linter
> script to trigger false positives".
Some observations perhaps worth considering:
The linter is happy (no false positives) with the 13000+ existing
tests (though, of course, not all of them use subshells). Those tests,
written over many years, vary quite wildly in style and implementation
approach, so the "subset" of shell language accepted by the linter is
quite broad.
The original --chain-lint series (jk/test-chain-lint) had to make some
changes, such as wrapping code in a {...} block[1], merely to pacify
the linter. v2 of the subshell linter required no such changes.
The subshell linter was crafted to be on par with the existing
--chain-lint in terms of strictness (and looseness), so the subshell
linter is not more strict than the existing implementation. (For
instance, one can escape the strict &&-chain requirement in the
existing --chain-lint by wrapping code in a {...} block. The subshell
linter intentionally allows that escape, as well.)
And, perhaps most important: We're not tied indefinitely to the
"subset" implemented by the current linter. If it is indeed found to
be too strict or limiting, it can always be loosened or retired
altogether.
Thanks for the feedback.
[1]: bfe998fc9b (t0050: appease --chain-lint, 2015-03-20)
