-
Notifications
You must be signed in to change notification settings - Fork 3
fixup #89 to get latest version #90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UtilModule as util.py
User->>UtilModule: Call _resolve_version(versions, user_input)
UtilModule->>UtilModule: Filter versions starting with user_input
alt No matches
UtilModule-->>User: Raise ValueError
else Matches found
UtilModule->>UtilModule: Parse versions to integer tuples
UtilModule->>UtilModule: Select max (latest) version
UtilModule-->>User: Return latest matching version
end
Warning Review ran into problems🔥 ProblemsGitHub checks timed out after 90 seconds. Some checks were still in progress when the timeout was reached. Consider increasing the 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (6)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #90 +/- ##
==========================================
+ Coverage 95.48% 95.65% +0.16%
==========================================
Files 3 3
Lines 133 138 +5
==========================================
+ Hits 127 132 +5
Misses 6 6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates version resolution logic to pick the latest matching version, adjusts related tests to expect the newest patch versions, and updates the CI success threshold in the run script.
- Switches
_resolve_version
to parse and select the maximum matching version. - Updates tests in
test_util.py
to assert on latest rather than first matching versions. - Adjusts
run.sh
to consider 9 failures as success instead of 10.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
tests/test_util.py | Updated expected version outputs to reflect latest patch versions |
testing/run.sh | Changed successful failure count from 10 to 9 |
cpp_linter_hooks/util.py | Enhanced _resolve_version to parse versions and return the latest matching release |
Comments suppressed due to low confidence (1)
testing/run.sh:28
- [nitpick] Use modern command substitution
$(...)
instead of backticks and quote variables (e.g.,failed_cases=$(grep -c "Failed" result.txt)
and laterif [ "$failed_cases" -eq 9 ]
) to improve readability and robustness.
failed_cases=`grep -c "Failed" result.txt`
return next(v for v in versions if v.startswith(user_input) or v == user_input) | ||
except StopIteration: | ||
# filter versions that start with the user input | ||
matched_versions = [v for v in versions if v.startswith(user_input)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This prefix match can unintentionally include versions like '20.10.0' when user_input is '20.1'. Consider matching on segment boundaries (e.g., v == user_input or v.startswith(user_input + '.')
) to avoid partial overlaps.
matched_versions = [v for v in versions if v.startswith(user_input)] | |
matched_versions = [v for v in versions if v == user_input or v.startswith(user_input + '.')] |
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
CodSpeed Performance ReportMerging #90 will improve performances by 10.35%Comparing Summary
Benchmarks breakdown
|
Summary by CodeRabbit
Bug Fixes
Tests
Chores