Hey,
I recently stumbled across shellcheck[1] and think we should make it a goal that our shell scripts don't produce errors/warnings with it.
Some of the examples in the GSM tester:
In jenkins-build-common.sh line 76: cd "$base" ^-- SC2164: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
In jenkins-build-common.sh line 93: rm -rf * ^-- SC2035: Use ./*glob* or -- *glob* so names with dashes won't become options.
What do you think? My approach would be:
* Install shellcheck into our build containers * Run them and provide warnings * Fix them over time and on new code * Make CI fail with failures
cheers holger
Hi Holger,
On Sat, Sep 15, 2018 at 08:35:48AM +0100, Holger Freyther wrote:
I recently stumbled across shellcheck[1] and think we should make it a goal that our shell scripts don't produce errors/warnings with it.
definitely a good idea!
What do you think? My approach would be:
- Install shellcheck into our build containers
- Run them and provide warnings
- Fix them over time and on new code
- Make CI fail with failures
sounds like a good plan. I suppose any changes to the build slaves would be performed via our ansible playbooks to ensure we can replicate the setup at any point in the future, as needed.
Thanks!
Hi Holger,
I personally have shellcheck plugin in my editor (atom) which performs checks in real time when I write shell scripts, I really recommend using it if your editor supports it.
Regarding checking stuff with shellcheck, it can be fine but it may be dangerous requiring fix of all warnings/errors, since some of them are sometimes really hard to fix. For instance doing some stuff in posix shell bs doing it for bash or whatever.
A related topic could be discussing about using "uncrustify" to force correct code format to be applied to code during gerrit jenkins job.
Regards, Pau
Hi Pau,
On 9/15/18 10:07 PM, Pau Espin Pedrol wrote:> Regarding checking stuff with shellcheck, it can be fine but it may be
dangerous requiring fix of all warnings/errors, since some of them are sometimes really hard to fix. For instance doing some stuff in posix shell bs doing it for bash or whatever.
I've been using shellcheck a lot myself, and in my experience it is actually feasible to require scripts having no warnings and errors. We even have that set up as a CI test in a project.
It detects the shell from the shebang, so if one specifies #!/bin/bash, it allows using the bash features. For warnings that are not helpful, it's possible to add a line like the following above:
# shellcheck disable=SC2086
Regards, Oliver
-
Harald Welte -
Holger Freyther -
Oliver Smith -
Pau Espin Pedrol