chore(shellcheck): add yarn shellcheck script and CI workflow#3300
Draft
kriskowal wants to merge 2 commits into
Draft
chore(shellcheck): add yarn shellcheck script and CI workflow#3300kriskowal wants to merge 2 commits into
kriskowal wants to merge 2 commits into
Conversation
Address every finding shellcheck emits at -S warning over the repository's tracked shell scripts. The changes are mechanical and do not alter runtime behavior: - Add missing `#!/bin/sh` / `#!/bin/bash` shebangs (SC2148). - Append `|| exit` to bare `cd` invocations whose failure should abort (SC2164). - Convert `for X in $(find ...)` to `while read; do ...; done < <(find ...)` so filenames with whitespace survive (SC2044). - Convert `find ... | xargs` to `find ... -print0 | xargs -0` for the same reason (SC2038). - Replace `CDPATH=` with `CDPATH=''` so the assignment is unambiguous (SC1007). - Drop an unused `DIR=$(dirname ...)` assignment (SC2034). - Add `-r` to `read` calls so backslashes round-trip through the pipe. These prepare the tree for the `yarn shellcheck` gate that lands in the same PR.
Adds a deterministic lint gate over every tracked .sh file in the repository. - `scripts/shellcheck.sh` enumerates `git ls-files '*.sh'` and runs `shellcheck -S warning` over the list, forwarding any extra args to the underlying checker. The file list is streamed through a git blob (`git hash-object -w --stdin` then `git cat-file blob $HASH`) rather than an argument vector or shell variable, keeping clear of ARG_MAX limits on hosts with many tracked .sh files. Exits 0 when the list is empty so forks or branches that have not yet adopted a shell script stay green. - `yarn shellcheck` wires the wrapper into the root `package.json` scripts table. - `.github/workflows/shellcheck.yml` runs the gate on push to master and on any pull_request that touches a `.sh` file (the workflow's `paths:` filter). A pull request that touches no shell scripts does not trigger the workflow at all. shellcheck ships preinstalled on `ubuntu-latest` runners so no additional dependency or apt install is needed. Action pins follow the project's existing zizmor-pedantic posture; the workflow scope grants `contents: read` and the job runs with `persist-credentials: false`.
|
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
1 participant
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.
Description
Adds a
yarn shellcheckgate that lint-checks every tracked shell script in the repository, and brings the existing scripts into compliance with it.The wrapper (
scripts/shellcheck.sh) enumeratesgit ls-files '*.sh'and runsshellcheck -S warningover the result, forwarding any extra arguments to the underlying checker. The file list is streamed to shellcheck through a git blob (git hash-object -w --stdin, thengit cat-file blob) rather than an argument vector or shell variable, so the gate stays clear ofARG_MAXon hosts with many tracked scripts. It exits 0 when the list is empty, so a fork or branch with no shell scripts stays green.A companion CI workflow runs the gate on push to
masterand on any pull request whose diff touches a.shfile; the workflow's path filter means a pull request that changes no shell scripts does not trigger it at all. shellcheck ships preinstalled on theubuntu-latestrunner, so the workflow needs no install step.The remainder of the change fixes the warnings shellcheck reports at
-S warningacross the scripts that already exist in the tree, so the gate is green from the first run. The fixes are mechanical and do not change runtime behavior: missing shebangs,|| exitoncdinvocations whose failure should abort, whitespace-safe iteration overfindoutput,read -r, and unambiguousCDPATH=''.Security Considerations
The workflow runs with
contents: readandpersist-credentials: false, and pins its actions to commit SHAs in keeping with the repository's existing posture. The gate executes shellcheck over the repository's own tracked scripts and grants no new authority to those scripts.Scaling Considerations
The workflow only runs when a pull request's diff includes a shell script, so it adds no CI time to the common case. When it does run, it is a single shellcheck pass over a small file set.
Documentation Considerations
None. The
yarn shellcheckscript is self-describing and the workflow is internal to CI.Testing Considerations
The gate is its own test: it passes against the current tree because this change also fixes the pre-existing warnings. New shell scripts (or edits to existing ones) that introduce a warning at
-S warningwill fail the gate in CI.Compatibility Considerations
No usage patterns change. The script edits preserve existing behavior; the new gate is additive.
Upgrade Considerations
None. This affects developer tooling and CI only, with no runtime or on-chain impact.