Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix shellcheck warnings in the scripts #118

Open
eero-t opened this issue Oct 6, 2022 · 2 comments
Open

Fix shellcheck warnings in the scripts #118

eero-t opened this issue Oct 6, 2022 · 2 comments

Comments

@eero-t
Copy link
Contributor

eero-t commented Oct 6, 2022

Pre-condition:

  • sudo apt install shellcheck or dnf install shellcheck

There are quite a few shell scripts:

find -name '*.*sh' | wc -l
38

And they trigger a lot of shellcheck warnings:

find -name '*.*sh' | xargs shellcheck | grep SC[0-9]*: | wc -l
658

Note: one should not follow shellcheck recommendations blindly. Main one where one could screw up is changing this:

base_opts="-a -b -c"
command $base_opts -d -e -f

To:

base_opts="-a -b -c"
command "$base_opts" -d -e -f

My preferred way is just adding shellcheck override for them, or switching to helper function like this:

run_command () {
   command -a -b -c $*
}
run_command -d -e -f

Whichever makes more sense for given code.

@eero-t
Copy link
Contributor Author

eero-t commented Aug 21, 2023

@dvrogozh any comments on this?

@dvrogozh
Copy link
Contributor

This issue is where help is clearly wanted. This task is in a backlog, but I can't say when we will have a chance to look into it ourselves.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants