Skip to content

Commit d40621c

Browse files
nammnlsierantlucian-tosa
authored
fix shellcheck and auto fix (#372)
# Summary This pull request focuses on improving shell script robustness and consistency across the codebase. The most significant change is an enhancement to the pre-commit hook's `shellcheck` integration, which now attempts to auto-fix issues and provides a clearer summary of failures. **Shellcheck automation and reporting:** * Enhanced the `.githooks/pre-commit` script to attempt auto-fixing shellcheck issues, collect failed files, and display a summary of failures instead of exiting on the first error. This improves developer experience and code quality. * We will not print each file we check, that is not useful information. Only print if we fixed one or were unable to fix it automatically ## Proof of Work - running `make precommit` locally - failing: ``` update_jobs: user 0m0.053s update_jobs: sys 0m0.042s lint_code: Go Version: go version go1.24.0 darwin/arm64 lint_code: Running golangci-lint... start_shellcheck: shellcheck failed on scripts/dev/recreate_python_venv.sh start_shellcheck: start_shellcheck: In scripts/dev/recreate_python_venv.sh line 80: start_shellcheck: sudo apt-get update -qq && sudo apt-get install -y python3-venv || true start_shellcheck: ^-- SC2015 (info): Note that A && B || C is not if-then-else. C may run when A is true. start_shellcheck: start_shellcheck: For more information: start_shellcheck: https://www.shellcheck.net/wiki/SC2015 -- Note that A && B || C is not if-t... Some checks have failed: start_shellcheck (PID 53467) To see the details look for the job's logs by it's prefixed name (e.g. "shellcheck failed" ``` - passing: ``` ython_formatting: Skipped 8 files python_formatting: formatting black python_formatting: All done! ✨ 🍰 ✨ python_formatting: 410 files left unchanged. real 0m11.694s user 0m2.030s sys 0m5.022s pre-commit: All checks passed! ... ``` ## Checklist - [ ] Have you linked a jira ticket and/or is the ticket in the title? - [ ] Have you checked whether your jira ticket required DOCSP changes? - [ ] Have you added changelog file? - use `skip-changelog` label if not needed - refer to [Changelog files and Release Notes](https://github.com/mongodb/mongodb-kubernetes/blob/master/CONTRIBUTING.md#changelog-files-and-release-notes) section in CONTRIBUTING.md for more details --------- Co-authored-by: Łukasz Sierant <[email protected]> Co-authored-by: Lucian Tosa <[email protected]>
1 parent 33bd4d9 commit d40621c

File tree

9 files changed

+25
-17
lines changed

9 files changed

+25
-17
lines changed

.githooks/pre-commit

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ wait_for_all_background_jobs() {
202202
for failure in "${failures[@]}"; do
203203
echo -e "$failure"
204204
done
205-
echo -e "${RED}To see the details look for the job's logs by it's prefixed name (e.g. \"shellcheck:\").${NO_COLOR}"
205+
echo -e "${RED}To see the details look/filter for the job's logs by it's prefixed name (e.g. \"start_shellcheck:\", \"lint_code:\", \"shellcheck failed\").${NO_COLOR}"
206206
return 1
207207
fi
208208

@@ -229,10 +229,17 @@ pre_commit() {
229229
# Function to run shellcheck on a single file
230230
run_shellcheck() {
231231
local file="$1"
232-
echo "Running shellcheck on $file"
233-
if ! shellcheck --color=always -x "$file" -e SC2154 -e SC1091 -e SC1090 -e SC2148 -o require-variable-braces -P "scripts"; then
232+
233+
local diff_output
234+
diff_output=$(shellcheck -f diff "$file" -e SC2154 -e SC1091 -e SC1090 -e SC2148 -o require-variable-braces -P "scripts" 2>&1)
235+
236+
if [[ -n "$diff_output" && "$diff_output" != *"Issues were detected, but none were auto-fixable"* ]]; then
237+
echo "$diff_output" | git apply
238+
echo "Applied auto-fixes for $file"
239+
elif [[ "$diff_output" == *"Issues were detected, but none were auto-fixable"* ]]; then
234240
echo -e "${RED}shellcheck failed on $file${NO_COLOR}"
235-
exit 1
241+
shellcheck --color=always -x "$file" -e SC2154 -e SC1091 -e SC1090 -e SC2148 -o require-variable-braces -P "scripts"
242+
return 1
236243
fi
237244
}
238245

scripts/dev/recreate_python_venv.sh

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ ensure_required_python() {
7777
# Install python3-venv package for Debian/Ubuntu systems if needed
7878
if command -v apt-get &> /dev/null; then
7979
echo "Installing python3-venv package for venv support..." >&2
80-
sudo apt-get update -qq && sudo apt-get install -y python3-venv || true
80+
sudo apt-get update -qq || true
81+
sudo apt-get install -y python3-venv || true
8182
fi
8283
return 0
8384
fi

scripts/dev/switch_context.sh

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,17 +105,17 @@ if [[ -n "${PROJECT_DIR:-}" && -x "${PROJECT_DIR}/bin/kubectl" ]]; then
105105
KUBECTL_CMD="${PROJECT_DIR}/bin/kubectl"
106106
fi
107107

108-
if [[ "$KUBECTL_CMD" != "kubectl" ]] || which kubectl > /dev/null; then
108+
if [[ "${KUBECTL_CMD}" != "kubectl" ]] || which kubectl > /dev/null; then
109109
if [ "${CLUSTER_NAME-}" ]; then
110110
# The convention: the cluster name must match the name of kubectl context
111111
# We expect this not to be true if kubernetes cluster is still to be created (minikube/kops)
112-
if ! "$KUBECTL_CMD" config use-context "${CLUSTER_NAME}"; then
112+
if ! "${KUBECTL_CMD}" config use-context "${CLUSTER_NAME}"; then
113113
echo "Warning: failed to switch kubectl context to: ${CLUSTER_NAME}"
114114
echo "Does a matching Kubernetes context exist?"
115115
fi
116116

117117
# Setting the default namespace for current context
118-
"$KUBECTL_CMD" config set-context "$("$KUBECTL_CMD" config current-context)" "--namespace=${NAMESPACE}" &>/dev/null || true
118+
"${KUBECTL_CMD}" config set-context "$("${KUBECTL_CMD}" config current-context)" "--namespace=${NAMESPACE}" &>/dev/null || true
119119

120120
# shellcheck disable=SC2153
121121
echo "Current context: ${context} (kubectl context: ${CLUSTER_NAME}), namespace=${NAMESPACE}"

scripts/evergreen/build_multi_cluster_kubeconfig_creator.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ chmod +x docker/mongodb-kubernetes-tests/multi-cluster-kube-config-creator_s390x
3333
chmod +x docker/mongodb-kubernetes-tests/multi-cluster-kube-config-creator_ppc64le
3434
chmod +x docker/mongodb-kubernetes-tests/multi-cluster-kube-config-creator_arm64
3535

36-
cp docker/mongodb-kubernetes-tests/multi-cluster-kube-config-creator_${ARCH} docker/mongodb-kubernetes-tests/multi-cluster-kube-config-creator || true
36+
cp docker/mongodb-kubernetes-tests/multi-cluster-kube-config-creator_"${ARCH}" docker/mongodb-kubernetes-tests/multi-cluster-kube-config-creator || true
3737

3838
mkdir -p bin || true
3939
cp docker/mongodb-kubernetes-tests/multi-cluster-kube-config-creator bin/kubectl-mongodb || true

scripts/evergreen/e2e/build_tests_image_ibm.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,6 @@ cp requirements.txt docker/mongodb-kubernetes-tests/requirements.txt
99
cp -rf helm_chart docker/mongodb-kubernetes-tests/helm_chart
1010

1111
echo "Building mongodb-kubernetes-tests image with tag: ${BASE_REPO_URL}/mongodb-kubernetes-tests:${version_id}-$(arch)"
12-
cd docker/mongodb-kubernetes-tests
12+
cd docker/mongodb-kubernetes-tests || exit
1313
sudo podman buildx build --progress plain . -f Dockerfile -t "${BASE_REPO_URL}/mongodb-kubernetes-tests:${version_id}-$(arch)" --build-arg PYTHON_VERSION="${PYTHON_VERSION}"
1414
sudo podman push --authfile="/root/.config/containers/auth.json" "${BASE_REPO_URL}/mongodb-kubernetes-tests:${version_id}-$(arch)"

scripts/evergreen/setup_aws.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ install_aws_cli_pip() {
5858
${pip_cmd} install --user awscli
5959

6060
# Add ~/.local/bin to PATH if not already there (where pip --user installs)
61-
if [[ ":$PATH:" != *":$HOME/.local/bin:"* ]]; then
62-
export PATH="$HOME/.local/bin:$PATH"
61+
if [[ ":${PATH}:" != *":${HOME}/.local/bin:"* ]]; then
62+
export PATH="${HOME}/.local/bin:${PATH}"
6363
echo "Added ~/.local/bin to PATH"
6464
fi
6565

scripts/evergreen/setup_kind.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ arch_suffix=$(detect_architecture)
1212
latest_version="v0.27.0"
1313

1414
# Only proceed with installation if architecture is supported (amd64 or arm64)
15-
if [[ "$arch_suffix" == "amd64" || "$arch_suffix" == "arm64" ]]; then
15+
if [[ "${arch_suffix}" == "amd64" || "${arch_suffix}" == "arm64" ]]; then
1616
mkdir -p "${PROJECT_DIR}/bin/"
1717
echo "Saving kind to ${PROJECT_DIR}/bin"
1818
curl --retry 3 --silent -L "https://github.com/kubernetes-sigs/kind/releases/download/${latest_version}/kind-${os}-${arch_suffix}" -o kind

scripts/evergreen/setup_kubectl.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ mv kubectl "${bindir}"
2222
echo "Downloading helm for ${ARCH}"
2323
helm_archive="${tmpdir}/helm.tgz"
2424
helm_version="v3.17.1"
25-
curl -s https://get.helm.sh/helm-${helm_version}-linux-${ARCH}.tar.gz --output "${helm_archive}"
25+
curl -s https://get.helm.sh/helm-${helm_version}-linux-"${ARCH}".tar.gz --output "${helm_archive}"
2626

2727
tar xfz "${helm_archive}" -C "${tmpdir}" &> /dev/null
2828
mv "${tmpdir}/linux-${ARCH}/helm" "${bindir}"

scripts/minikube/setup_minikube.sh

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ EOF
2929
}
3030

3131
# retrieve arch variable off the shell command line
32-
ARCH=${1-"$(detect_architecture)"}
32+
ARCH="$(detect_architecture)"
3333

3434
echo "Setting up minikube host for architecture: ${ARCH}"
3535

@@ -50,7 +50,7 @@ setup_local_registry_and_custom_image() {
5050
registry_running=true
5151
fi
5252

53-
if ! $registry_running; then
53+
if ! ${registry_running}; then
5454
echo "Starting local container registry on port 5000..."
5555

5656
# Clean up any existing registry first
@@ -63,7 +63,7 @@ setup_local_registry_and_custom_image() {
6363

6464
# Wait for registry to be ready
6565
echo "Waiting for registry to be ready..."
66-
for i in {1..30}; do
66+
for _ in {1..30}; do
6767
if curl -s http://localhost:5000/v2/_catalog >/dev/null 2>&1; then
6868
break
6969
fi

0 commit comments

Comments
 (0)