fix: grep -c arithmetic error in quality sweep ShellCheck section#2637
fix: grep -c arithmetic error in quality sweep ShellCheck section#2637marcusquinn merged 1 commit intomainfrom
Conversation
grep -c outputs '0' on no match AND exits 1. The || echo '0' then appends a second '0', giving '0\n0' which fails in $(( )). Fix: use || var=0 as a separate statement instead of piped fallback.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical bug in the daily quality sweep script that was causing it to fail due to an arithmetic syntax error. The change modifies how the count of ShellCheck errors and warnings is handled when Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
WalkthroughModified ShellCheck finding counting in the pulse-wrapper script by adjusting the grep pattern's capitalization and spacing, and relocated the default-fallback logic from an inline echo to an explicit shell-guarded assignment for improved default value handling. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sun Mar 1 04:42:35 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly fixes a bug in the quality sweep script where grep -c was causing an arithmetic error on no match. The change to use || var=0 is a robust way to handle the exit code of grep while set -e is active. I have a suggestion to improve the code further by using a here-string for efficiency and to separate the local declaration from assignment for file_warnings for consistency and safety.
| file_errors=$(echo "$result" | grep -c ':.*: error:') || file_errors=0 | ||
| local file_warnings | ||
| file_warnings=$(echo "$result" | grep -c ':.*: warning:' || echo "0") | ||
| file_warnings=$(echo "$result" | grep -c ':.*: warning:') || file_warnings=0 |
There was a problem hiding this comment.
While the fix is correct, for better performance and to adhere to modern shell scripting best practices, consider using a here-string (<<<) to pass the variable to grep. This avoids creating an extra subshell for echo and can be more efficient, especially if this script is run in a loop over many files. Additionally, for consistency and adherence to best practices, the local declaration for file_warnings has been separated from its assignment, as command substitution is involved.
| file_errors=$(echo "$result" | grep -c ':.*: error:') || file_errors=0 | |
| local file_warnings | |
| file_warnings=$(echo "$result" | grep -c ':.*: warning:' || echo "0") | |
| file_warnings=$(echo "$result" | grep -c ':.*: warning:') || file_warnings=0 | |
| file_errors=$(grep -c ':.*: error:' <<< "$result") || file_errors=0 | |
| local file_warnings | |
| file_warnings=$(grep -c ':.*: warning:' <<< "$result") || file_warnings=0 |
References
- When declaring and assigning local variables in shell scripts, separate the
localdeclaration from the assignment if command substitution is involved, to ensure exit code safety. For consistency, this pattern should be maintained even for simple assignments.
1 participant
Summary
syntax error in expression (error token is "0\n0")that prevented the daily quality sweep from completinggrep -coutputs0on no match AND exits 1. With|| echo "0", output becomes0\n0(two lines) which fails in$(( ))arithmeticfile_errors=$(... | grep -c ...) || file_errors=0— the fallback assigns directly instead of appending to stdoutSummary by CodeRabbit