fix(setup): '//' is not a shell comment - #373
Conversation
Line 3 used a C-style '//' comment in a bash script. '//' is not a comment — bash tries to execute it, so the script emitted a 'No such file or directory' error on every run and shellcheck could not parse the file at all. Changed to '#'. Found by an estate-wide shellcheck sweep of 5,111 tracked scripts across 375 repos. This file was one of 11 that fail to PARSE (SC1073/SC1072) — shellcheck stops analysing at the failure, so anything after it was never checked either. Verified: shellcheck -S error now reports 0 findings for this file.
|
Warning Review limit reachedNext included review available in 55 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✨ 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 |
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 correctly identifies and addresses a critical syntax error in setup.sh where C-style comments (//) were used. In a shell environment, this would cause a runtime failure as the shell attempts to execute // as a command; this is particularly dangerous here due to the set -e directive, which would trigger an immediate termination of the script. Although the change aligns with the requirements, the PR lacks evidence of validation. It is essential to confirm that the script passes ShellCheck and executes correctly without parsing errors before it is merged.
Test suggestions
- Run ShellCheck on setup.sh to ensure zero parsing errors.\n- [ ] Execute the script to verify that line 3 no longer produces a 'No such file or directory' error.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Run ShellCheck on setup.sh to ensure zero parsing errors.\n- [ ] Execute the script to verify that line 3 no longer produces a 'No such file or directory' error.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| #!/usr/bin/env bash | ||
| # SPDX-License-Identifier: MPL-2.0 | ||
| // Owner: Jonathan D.A. Jewell <j.d.a.jewell@open.ac.uk> | ||
| # Owner: Jonathan D.A. Jewell <j.d.a.jewell@open.ac.uk> |
There was a problem hiding this comment.
🟡 MEDIUM RISK
Suggestion: The invalid // comment prefix must be replaced with # to ensure shell compatibility. The previous use of // would have caused a runtime error, which is fatal to the script given the set -e flag in the preamble.\n\nsuggestion\n# Owner: Jonathan D.A. Jewell <j.d.a.jewell@open.ac.uk>\n
1 participant
Line 3 used a C-style '//' comment in a bash script. '//' is not a comment — bash tries to execute it, so the script emitted a 'No such file or directory' error on every run and shellcheck could not parse the file at all. Changed to '#'.
Found by an estate-wide shellcheck sweep of 5,111 tracked scripts across 375 repos. This file was one of 11 that fail to parse (
SC1073/SC1072) — shellcheck stops analysing at the failure, so everything after it was never checked either.Verified:
shellcheck -S errornow reports 0 findings for this file.