Skip to content

Apply ShellCheck fixes to bash scripts#663

Open
frankgrimes97 wants to merge 1 commit intoapache:masterfrom
frankgrimes97:fix/apply-shellcheck-fixes-to-bash-scripts
Open

Apply ShellCheck fixes to bash scripts#663
frankgrimes97 wants to merge 1 commit intoapache:masterfrom
frankgrimes97:fix/apply-shellcheck-fixes-to-bash-scripts

Conversation

@frankgrimes97
Copy link
Member

@frankgrimes97 frankgrimes97 commented Mar 11, 2026

Fixes #662

Motivation

ShellCheck flagged a number of issues with the Bash scripts some of which could lead to unexpected behaviour.
e.g.

In scripts/pulsar/prepare_helm_release.sh line 21:
CHART_HOME=$(unset CDPATH && cd $(dirname "${BASH_SOURCE[0]}")/../.. && pwd)
                                ^----------------------------^ SC2046 (warning): Quote this to prevent word splitting.

Modifications

I've attempted to address most of the issues reported by ShellCheck 0.11.0.
For other seeming false positives I suppressed them using ShellCheck ignore directives

e.g. https://www.shellcheck.net/wiki/SC2016

$ grep shellcheck -A1 .ci/helm.sh 
# shellcheck disable=SC2034
KIND_BIN=$OUTPUT_BIN/kind
--
      # shellcheck disable=SC2016
      timeout 120s "${KUBECTL}" exec -n "${NAMESPACE}" "${CLUSTER}"-toolset-0 -- bash -c 'until [ "$(curl -s -L http://pulsar-ci-broker:8080/status.html)" == "OK" ]; do sleep 3; done' || { echo >&2 "Timeout waiting..."; ci::print_pod_logs; exit 1; }

Verifying this change

I'm not sure how exhaustively these scripts are covered by CI tests and haven't manually tested all of them due to my lack of familiarity with the project so some guidance/assistance may be required.

@frankgrimes97
Copy link
Member Author

I see the CI checks show a lot of red... I'll try to dig in and see how my changes might have broken things. 😄

ShellCheck: https://github.com/koalaman/shellcheck

Also suppressed a few ShellCheck warnings where deemed necessary.
@frankgrimes97 frankgrimes97 force-pushed the fix/apply-shellcheck-fixes-to-bash-scripts branch from 942a09f to c7a87b6 Compare March 12, 2026 13:53
@frankgrimes97
Copy link
Member Author

CI is back to green now so I think this should be ready for review.

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.

Cleanup Bash scripts

1 participant