shellcheck for our shell scripts?
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 [1] https://www.shellcheck.net/
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! -- - Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/ ============================================================================ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6)
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 -- - Pau Espin Pedrol <pespin@sysmocom.de> http://www.sysmocom.de/ ======================================================================= * sysmocom - systems for mobile communications GmbH * Alt-Moabit 93 * 10559 Berlin, Germany * Sitz / Registered office: Berlin, HRB 134158 B * Geschaeftsfuehrer / Managing Director: Harald Welte
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 -- - Oliver Smith <osmith@sysmocom.de> https://www.sysmocom.de/ ======================================================================= * sysmocom - systems for mobile communications GmbH * Alt-Moabit 93 * 10559 Berlin, Germany * Sitz / Registered office: Berlin, HRB 134158 B * Geschaeftsfuehrer / Managing Director: Harald Welte
participants (4)
-
Harald Welte -
Holger Freyther -
Oliver Smith -
Pau Espin Pedrol