fix(shell): declare the shell — shebang where executed, directive where sourced - #49
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. (2)
🔇 Additional comments (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe shell hook now declares its Bash dialect for ShellCheck. No runtime behaviour or exported entities changed. ChangesShellCheck directive
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to The PR adds a shell declaration to one sourced script without changing its execution behavior. No actionable merge-blocking risk remains, so it is merge-ready after normal checks and review. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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
The PR resolves the SC2148 ShellCheck warning in shell/coord-hooks.sh by applying a shell directive. This ensures correct dialect recognition for sourced files in accordance with the project's execution model policy. All automated quality and logic checks passed without any identified issues or risks.
Test suggestions
- Run shellcheck on
shell/coord-hooks.shto ensure SC2148 is resolved and the dialect is correctly recognized. - Verify the script remains compatible when sourced in a Bash environment.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Run shellcheck on `shell/coord-hooks.sh` to ensure SC2148 is resolved and the dialect is correctly recognized.
2. Verify the script remains compatible when sourced in a Bash environment.
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