fix: avoid appending --extra-arg= for clang-tidy#172
Conversation
This avoids adding a `--extra-arg=` (with no arg value) to clang-tidy invocations.
WalkthroughA minor bug fix in the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
![coderabbitai[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cpp_linter/clang_tools/clang_tidy.py (1)
249-252: LGTM! The fix correctly prevents empty arguments from being passed to clang-tidy.The conditional check successfully filters out empty strings after quote stripping, which addresses the issue described in the PR.
Optional: Consider also stripping whitespace for more defensive handling
If you want to handle whitespace-only arguments more defensively (e.g.,
" "after quote removal), you could also strip whitespace:for extra_arg in extra_args: - arg = extra_arg.strip('"') + arg = extra_arg.strip('"').strip() if arg: # avoid adding empty arg values cmds.append(f"--extra-arg={arg}")This would prevent
--extra-arg=(with only whitespace) from being added. However, this is likely an edge case and the current implementation is sufficient for the stated goal.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cpp_linter/clang_tools/clang_tidy.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: test (ubuntu-latest, 9)
- GitHub Check: test (ubuntu-latest, 10)
- GitHub Check: test (ubuntu-latest, 15)
- GitHub Check: test (windows-latest, 18)
- GitHub Check: test (windows-latest, 13)
- GitHub Check: test (windows-latest, 16)
- GitHub Check: test (windows-latest, 17)
- GitHub Check: test (windows-latest, 15)
- GitHub Check: test (ubuntu-latest, 17)
- GitHub Check: test (windows-latest, 9)
- GitHub Check: test (ubuntu-latest, 18)
- GitHub Check: test (windows-latest, 11)
- GitHub Check: test (ubuntu-latest, 14)
- GitHub Check: test (ubuntu-latest, 21)
- GitHub Check: test (ubuntu-latest, 16)
- GitHub Check: test (windows-latest, 14)
- GitHub Check: test (windows-latest, 19)
- GitHub Check: test (windows-latest, 20)
- GitHub Check: test (windows-latest, 10)
- GitHub Check: test (windows-latest, 21)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #172 +/- ##
=======================================
Coverage 98.25% 98.26%
=======================================
Files 23 23
Lines 1896 1897 +1
=======================================
+ Hits 1863 1864 +1
Misses 33 33 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
This avoids adding a
--extra-arg=(with no arg value) to clang-tidy invocations.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.