Shellcheck warnings #52
Conversation
And not lower severities like info and style. For the moment, there are too many info messages, e.g. not using double quotes everywhere or having functions that are not directly called. Let's not push people to fix those and create massive cleanup patches not fixing actual problems. Signed-off-by: Matthieu Baerts (NGI0) <[email protected]>
If a file was shellcheck compliant before or is new, it is interesting to tell the reviewers when this status changes or if the new file is not shellcheck compliant from the beginning. A new dedicated file is used for each modified shell script, using a hash of the file path to cope with files with the same base name. If the status is different than before, warnings + info + style are added to the errors count. Note: it is not clear if the log message about a file no longer being shellcheck compliant any more should be more visible or not, i.e. sent to $DESC_FD. Signed-off-by: Matthieu Baerts (NGI0) <[email protected]>
Before, in case of new errors and new warnings, the script was exiting with rc=250 (warning) instead of rc=1 (error). This is fixed simply by looking at the warnings counter before the errors one, so rc=1 will be set last. Signed-off-by: Matthieu Baerts (NGI0) <[email protected]>
It sounds good to have the script checking for shellcheck compliance to
be shellcheck compliant :)
While at it, pylint and yamllint scripts have also be modified because
they are inspired by the shellcheck one.
Before the modifications, shellcheck was complaining about:
53 note: Double quote to prevent globbing and word splitting. [SC2086]
4 warning: Quote this to prevent word splitting. [SC2046]
3 error: Argument mixes string and array. Use * or separate argument. [SC2145]
2 warning: Use 'cd ... || exit' or 'cd ... || return' in case cd fails. [SC2164]
Not any more.
Also fix the indentation around the 'diff' at the end of each script,
and in shellcheck.sh, s/Building/Checking/ like the others.
Signed-off-by: Matthieu Baerts (NGI0) <[email protected]>
|
Thanks! |
|
@kuba-moo thank you for having reviewed and merged this, and for your reply! I wanted to see if everything was working as expected, and I noticed the CI doesn't have https://netdev.bots.linux.dev/static/nipa/968732/14107435/shellcheck/stderr Can you add it please? Ideally the last version. I didn't check if pylint and yamllint were installed. |
|
Thanks a lot for flagging that! I installed them on the wrong AWS instance 🤦 |
Great, now you can check if your quickly written/hack scripts for hot debugging you added on a production server are shellcheck compliant :-D |
Here are a few modifications around the new test scripts: