fix(shell): declare the shell — shebang where executed, directive where sourced - #319
Conversation
…re sourced
shellcheck SC2148 ('target shell is unknown') on 1 files here.
Deliberately NOT a blanket shebang. The files split by how they are USED:
* 0 executed (have +x, never source'd) -> added '#!/usr/bin/env bash'
* 1 sourced (referenced by source/.) -> added '# shellcheck shell=bash'
A source'd file is never run as its own process, so a shebang there is
misleading — it claims an execution model the file does not have. The
shellcheck directive states the dialect without making that false claim.
Found by an estate-wide sweep of 5,111 tracked scripts across 375 repos: 85
files lack a shell declaration. 40 of those were left alone deliberately —
they are vendored duplicates (7 identical copies of one recoverer.sh) or a
nested kith/ tree, and 37 of the 40 are never invoked by name anywhere, so
editing them would be churn in vendored code.
Remaining SC2148 in this repo after the change: 0
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🔇 Additional comments (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change adds a Bash ShellCheck directive to ChangesShellCheck declaration
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This localized change declares the Bash dialect for a sourced script and introduces no actionable merge-blocking risk; it is merge-ready after normal checks and review. Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description explains the change, its rationale, scope, and the resulting SC2148 status. However, it does not follow the repository template and omits the required checklist, explicit Changes section, Testing section, and Screenshots section. Resolution Update the description to use the repository template. Add the Summary, Changes, RSR Quality Checklist, and Testing sections. Mark applicable checklist items and state the exact validation commands or results. Include Screenshots only if applicable. Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Up to standards ✅🟢 Issues
|
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull Request Overview
This PR resolves the ShellCheck SC2148 warning ('No shebang found') by introducing the '# shellcheck shell=bash' directive to 'coord-tui/shell/coord-hooks.sh'. This approach correctly identifies the shell dialect for sourced files without incorrectly implying they are standalone executables.
The change aligns with shell scripting best practices and addresses the linting gaps identified in the acceptance criteria. No blockers or regressions were detected.
Test suggestions
- Verify that ShellCheck successfully identifies the dialect as Bash for coord-hooks.sh and no longer reports SC2148.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify that ShellCheck successfully identifies the dialect as Bash for coord-hooks.sh and no longer reports SC2148.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
1 participant
shellcheck
SC2148(target shell is unknown) on 1 files here.Deliberately not a blanket shebang. The files split by how they are used:
+x, neversourced)#!/usr/bin/env bashsource/.)# shellcheck shell=bashA
sourced file is never run as its own process, so a shebang there is misleading — it claims an execution model the file does not have. The shellcheck directive states the dialect without making that false claim.Found by an estate-wide sweep of 5,111 tracked scripts across 375 repos: 85 files lack a shell declaration. 40 were left alone deliberately — vendored duplicates (7 identical copies of one
recoverer.sh) or a nestedkith/tree, and 37 of the 40 are never invoked by name anywhere, so editing them would be churn in vendored code.Remaining
SC2148in this repo after the change: 0