Fix all shellcheck warnings and add shellcheck to CI#146
Merged
Conversation
Resolved all 292 shellcheck warnings (--severity=warning) across 82
shell scripts. Zero warnings remain.
Main fix categories:
- SC2046 (106): Quote $(dirname "$0") expansions in command positions
- SC2034 (66): Suppress or remove genuinely unused variables
- SC2068 (15): Double-quote array expansions in for loops
- SC2164 (15): Add || exit to cd calls
- SC2178/SC2206/SC2128 (36): Fix array vs. string type mismatches
- SC2145 (5): Fix string+array argument mixing; use ${arr[*]}
- SC2166 (4): Replace -a/-o with && / || in [ ] tests
- SC2207 (6): Replace arr=($(...)) with mapfile -t
- SC2155 (4): Separate declare and assign
- SC2071 (1): Replace < with -lt in numeric [[ ]] test
- SC2048 (2): Replace $* with "$@"
- SC2053 (2): Quote RHS of == in [[ ]]
- SC1083 (2): Fix literal braces in awk (quoting issue)
- SC2061 (1): Quote -name glob argument in find
- SC2027 (3): Fix quoted concatenation
- Plus SC2154, SC2120, SC2165, SC2167, SC3010, SC3014, SC2043
Add ShellCheck step to .github/workflows/CI.yml so all future PRs
are checked automatically.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resolved all 1339 shellcheck info-level issues across 116 shell scripts. Zero issues remain at --severity=info. Main fix categories: - SC2086 (1259): Quote single-value variables; suppress intentional word-splits with inline disable comments - SC2012 (26): Replace ls-based file discovery with find - SC1091 (24): Add shellcheck source=/dev/null before sourced files - SC2196 (11): Replace deprecated egrep with grep -E - SC2153 (6): Fix variable name typos / suppress false positives - SC2021 (4): Fix [A-z] character ranges to [A-Za-z] - SC2162 (2): Add -r flag to read commands - SC2231 (2): Quote expansions in case patterns - SC2030/SC2031 (2): Fix subshell variable visibility bug in RUNLIST.whichRunsAreAtmosphere.sh - SC2013 (1): Replace for-in-$(cat) with while read loop - SC2269 (1): Remove self-assignment - SC2329 (1): Fix always-true expression Update CI to use --severity=info (stricter than the previous --severity=warning). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…k-info V492 dev18 shellcheck info
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes all 292 ShellCheck warnings (severity=warning) across 82 shell scripts in the repository and adds a ShellCheck step to the CI workflow so future contributions are automatically checked. The bulk of changes are mechanical: quoting of word-splitting expansions, replacing backticks with $(...), replacing arr=($(...)) with mapfile -t, switching to arrays for option lists, replacing egrep/-a/-o with portable equivalents, and adding || exit after cd.
Changes:
- Add ShellCheck job to
.github/workflows/CI.ymlrunningfind scripts -name "*.sh" | xargs shellcheck --severity=info. - Quote variable expansions and command substitutions, modernize backtick syntax, and convert option strings to arrays for safe word-splitting.
- Several latent fixes: variable renames in nested loops in
IRF.merge_and_link_MC_datasets.sh, fix of undefined$OFILE(now$TABFILE) inIRF.lookup_table_parallel_sub.sh, removal of unused variables.
Reviewed changes
Copilot reviewed 121 out of 121 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/CI.yml |
Add ShellCheck step to CI. |
scripts/ANALYSIS.*.sh, scripts/IRF.*.sh, scripts/SPANALYSIS.*.sh, scripts/RUNLIST.*.sh, scripts/UTILITY.*.sh, scripts/set_environment.sh |
Mechanical ShellCheck fixes (quoting, $(), arrays, || exit). |
scripts/preprocessing/*.sh |
ShellCheck fixes plus minor refactors (e.g., print_dqm_string_from_runlist.sh removes a no-op outer loop). |
scripts/db_scripts/*.sh |
Quote $EVNDISPSCRIPTS and $EVNDISPSYS in command paths; egrep → grep -E. |
scripts/helper_scripts/*.sh |
ShellCheck fixes; convert option strings to arrays; IRF.lookup_table_parallel_sub.sh switches $OFILE.list (undefined) to $TABFILE.list; IRF.optimizeTMVAforGammaHadronSeparation_sub.sh adds an unexplained rm -f "${MVADIR}/rates.log". |
scripts/IRF.optimizeTMVAforGammaHadronSeparation.sh |
ShellCheck fixes plus removal of unused EBINMIN/EBINMAX loops and unused DISPBDT/ANATYPE block. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolved all 292 shellcheck warnings (--severity=warning) across 82 shell scripts. Zero warnings remain.
Main fix categories:
Add ShellCheck step to .github/workflows/CI.yml so all future PRs are checked automatically.