refactor: fix shellcheck for templates#233
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR updates three shell script templates to properly handle shellcheck by disabling it around the Jinja-templated header comments, reducing false positives while preserving linting for the rest of the script content. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider using a more targeted
# shellcheck disable=…list for the specific template-related warnings rather thandisable=all, so that shellcheck can still catch real issues in the header section of these scripts.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider using a more targeted `# shellcheck disable=…` list for the specific template-related warnings rather than `disable=all`, so that shellcheck can still catch real issues in the header section of these scripts.
## Individual Comments
### Comment 1
<location> `templates/module-setup.sh.j2:2-3` </location>
<code_context>
#!/bin/sh
+# This is a template, not an actual shell script, so tell shellcheck to
+# ignore the problematic templated parts
+# shellcheck disable=all
{{ ansible_managed | comment }}
{{ "system_role:nbde_client" | comment(prefix="", postfix="") }}
</code_context>
<issue_to_address>
**suggestion:** Clarify wording so the generated script is not described as “not an actual shell script”.
Since this header will appear in an actual executable script, calling it “not an actual shell script” is misleading for anyone reading the generated file. Please reword to something like: “This file is generated from a template; shellcheck is disabled for the templated lines below,” or similar, so the comment is accurate in both the template and the rendered script.
```suggestion
# This file is generated from a template; disable shellcheck for the templated
# lines below so the template directives do not trigger shellcheck warnings
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| # This is a template, not an actual shell script, so tell shellcheck to | ||
| # ignore the problematic templated parts |
There was a problem hiding this comment.
suggestion: Clarify wording so the generated script is not described as “not an actual shell script”.
Since this header will appear in an actual executable script, calling it “not an actual shell script” is misleading for anyone reading the generated file. Please reword to something like: “This file is generated from a template; shellcheck is disabled for the templated lines below,” or similar, so the comment is accurate in both the template and the rendered script.
| # This is a template, not an actual shell script, so tell shellcheck to | |
| # ignore the problematic templated parts | |
| # This file is generated from a template; disable shellcheck for the templated | |
| # lines below so the template directives do not trigger shellcheck warnings |
Signed-off-by: Rich Megginson <rmeggins@redhat.com>
600661c to
be79ad3
Compare
Signed-off-by: Rich Megginson rmeggins@redhat.com
Summary by Sourcery
Enhancements: