On 16/06/2025 21:53, Petr Machata wrote: > > Matthieu Baerts <matt...@kernel.org> writes: > >> Hi Petr, >> >> On 16/06/2025 17:06, Petr Machata wrote: >>> >>> Matthieu Baerts <matt...@kernel.org> writes: > >>>> That what we did with MPTCP, see the top of the mptcp_join.sh file for >>>> example [2], where we have: >>>> >>>>> # ShellCheck incorrectly believes that most of the code here is >>>>> unreachable >>>>> # because it's invoked by variable name, see how the "tests" array is used >>>>> #shellcheck disable=SC2317 >>>> >>>> If you add this at the top of your new file, followed by an empty line, >>>> shellcheck will ignore this issue for the whole file. >>> >>> The ALL_TESTS issue is not the end of it either. We use helpers that >>> call stuff indirectly all over the place. defer, in_ns... It would make >>> sense to me to just disable SC2317 in NIPA runs. Or maybe even put it in >>> net/forwarding/.shellcheckrc. Pretty much all those tests are written in >>> a style that will hit these issues. >> >> In this case, I think it would be better to add this .shellcheckrc file >> in net/forwarding. If you modify NIPA, I don't think people will know >> what is allowed or not, or what command line to use, no? > > Good point. The question then is whether to put it to forwarding/ or > directly net/, which is has seen more use of lib.sh and therefore the > same sort of coding style. I'll experiment with it and should be able to > send it later in the week. I don't want to add it to the MC patchset.
Since Jakub disabled SC2317 in NIPA [1], then I guess we can put this .shellcheckrc file in net/, no? >> Note that NIPA's shellcheck reports are currently ignoring all "info" >> and "style" severities -- so including SC2317 -- except for new files or >> the ones that were previously shellcheck compliant. > > Yeah, I know, but the result is still very noisy, if you want to verify > it prior to sending the patchset. It's a bit annoying to have to scroll > through the report trying to find relevant stuff. I could add > .shellcheckrc in my own clone, but everybody is going to hit these. When Shellcheck got introduced, there were some discussions about SC2317 and SC2086 [2]. I don't think they are useless -- maybe a function is not used by mistake, maybe double quotes are really needed for some cases, etc. -- but I agree with you: they create a lot of noise. [1] https://github.com/linux-netdev/nipa/commit/23b74dd [2] https://github.com/linux-netdev/nipa/pull/51#issuecomment-2939556291 Cheers, Matt -- Sponsored by the NGI0 Core fund.
