Skip to content

Simplify shellcheck discovery to .sh only (review feedback from #6) - #7

Merged
dolph merged 3 commits into
mainfrom
claude/simplify-shellcheck-glob
Apr 26, 2026
Merged

Simplify shellcheck discovery to .sh only (review feedback from #6)#7
dolph merged 3 commits into
mainfrom
claude/simplify-shellcheck-glob

Conversation

@dolph

@dolph dolph commented Apr 26, 2026

Copy link
Copy Markdown
Owner

Summary

Addresses the review comment on #6 (#6 (comment)):

We can safely assume all shell files in this repo are .sh

Drops the multi-extension glob (*.bash, *.ksh, *.zsh) and the shebang-scan fallback. Replaces the bash-specific mapfile + read loop with a one-liner: git ls-files '*.sh' | xargs -0 -r shellcheck. The -r (no-run-if-empty) flag handles the empty case without an explicit guard.

The behavior the previous version was designed to preserve is unchanged: scripts in any subdirectory are still picked up, and newly added or renamed .sh files are linted automatically on the PR that introduces them — that property comes from git ls-files, not from the extension list.

Diff

-    - name: Find and lint shell scripts
-      shell: bash
-      run: |
-        set -euo pipefail
-        # ...22 lines of mapfile + shebang detection...
-        shellcheck -- "${scripts[@]}"
+    - name: Find and lint shell scripts
+      run: |
+        set -euo pipefail
+        echo "Discovered shell scripts:"
+        git ls-files -- '*.sh' | sed 's/^/  /'
+        git ls-files -z -- '*.sh' | xargs -0 -r shellcheck --

Net -23 +6 lines.

Test plan

  • Locally: discovers build.sh and install.sh, shellcheck exits 0.
  • CI run on the PR shows the new (shorter) shellcheck step passing.

https://claude.ai/code/session_013HnepY8MhhxrJJjE5ysW47


Generated by Claude Code

Per review feedback on #6: this repo's convention is that all shell
scripts are named *.sh, so the multi-extension globs and the shebang
scan are unnecessary. Drops the bash-specific `mapfile` + read loop
in favor of `git ls-files '*.sh' | xargs -0 -r shellcheck`, which is
shorter, POSIX-friendly, and still picks up scripts in any directory
on the PR that adds or renames them.

Resolves: #6 (comment)
Comment thread .github/workflows/go.yml Outdated
Comment thread .github/workflows/go.yml Outdated
@dolph
dolph merged commit 3d0216b into main Apr 26, 2026
2 checks passed
@dolph
dolph deleted the claude/simplify-shellcheck-glob branch April 26, 2026 23:01
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.

2 participants