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

GHA: add shellcheck job and fix warnings, shell tidy-ups #13307

Closed
wants to merge 28 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Apr 7, 2024

@vszakats vszakats added build tidy-up CI Continuous Integration labels Apr 7, 2024
@github-actions github-actions bot added the tests label Apr 7, 2024
@vszakats vszakats changed the title GHA: add shellcheck job and fix fallouts, tidy-up GHA: add shellcheck job and fix warnings, tidy-up Apr 7, 2024
scripts/contributors.sh Outdated Show resolved Hide resolved
@vszakats vszakats changed the title GHA: add shellcheck job and fix warnings, tidy-up GHA: add shellcheck job and fix warnings, tidy-ups Apr 8, 2024
@vszakats vszakats changed the title GHA: add shellcheck job and fix warnings, tidy-ups GHA: add shellcheck job and fix warnings, shell tidy-ups Apr 8, 2024
@vszakats vszakats closed this in fa69b41 Apr 8, 2024
@vszakats vszakats deleted the ci-shellcheck branch April 8, 2024 09:38
vszakats added a commit that referenced this pull request Apr 11, 2024
- use `$()` instead of backticks, and re-arrange double-quotes inside.
- add missing `|| exit 1` to `cd` calls. (could be dropped by using `set -eu`.)
- add `-n` to a few `if`s.
- shorten redirections by using `{} >` (as shellcheck recommended).
- silence warnings where variables were detected as unused (SC2034).
- a couple misc updates to silence warnings.
- switch to bash shebang for `-ot` feature.
- split two lines to unbreak syntax highlighting in my editor. (`$(expr \`, `$(dirname \`)

Also enable CI checks for OS/400 shell scripts.

Ref: #13307
Closes #13309
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants