Skip to content

shellcheck: add workflow, shellcheck fix the scripts #14905

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

yarikoptic
Copy link
Contributor

Request to shellcheck contribution was given in #13284 recently. Project has some shell scripts but they are not checked by CI . This PR does both - adds CI, and

Individual commits would have more explanation and provenance on changes.

  • add tox.ini env for shellcheck? Question: what to do with dependency on binary which isn't python project. There is https://pypi.org/project/shellcheck-py/ which says "A python wrapper to provide a pip-installable shellcheck binary." but I have no experience with it/not sure of gotchas. If told to use/add to requirements-devel.txt -- can do ;)

=== Do not change lines below ===
{
 "chain": [],
 "cmd": "shellcheckit doit -f diff | git apply",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [],
 "pwd": "."
}
^^^ Do not change lines above ^^^
codespell complained

	In misc/test-stubgenc.sh line 15:
	rm -rf $STUBGEN_OUTPUT_FOLDER/*
		   ^----------------------^ SC2115 (warning): Use "${var:?}" to ensure this never expands to /* .

	For more information:
	  https://www.shellcheck.net/wiki/SC2115 -- Use "${var:?}" to ensure this nev...

so I followed the recommendation and also ""quoted the value use later in the lines.
In principle that is all not required ATM since the value is hardcoded in the script BUT
happen the value changes, and e.g. variable name gets changed or mystyped without correct
changes to uses -- someone might end up in pain while triggering all those gotchas.  With
these changes code becomes safer and thus preferable imho.
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

Successfully merging this pull request may close these issues.

1 participant