fix: make sure git repo has commit before checking #253
Conversation
WalkthroughThe changes introduce early exit checks using the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CheckFunction
participant Util_has_commits
User->>CheckFunction: Run check (e.g., check_commit_signoff)
CheckFunction->>Util_has_commits: has_commits()
alt No commits
Util_has_commits-->>CheckFunction: False
CheckFunction-->>User: Return PASS (skip check)
else Commits exist
Util_has_commits-->>CheckFunction: True
CheckFunction->>CheckFunction: Continue with regular validation
end
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ 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). (5)
✨ 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 #253 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 7 7
Lines 298 300 +2
=========================================
+ Hits 298 300 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](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)
commit_check/commit.py (1)
58-63: Consider consistent behavior with other check functions.The function correctly handles empty repositories by returning
PASSearly, but it's the only one that prints a warning message. Other check functions (check_commit_msg,check_author,check_merge_base) silently returnPASSwithout any output.For consistency, consider either:
- Removing the warning message to match other functions
- Adding similar warning messages to other check functions
Current implementation:
if has_commits() is False: print( f"{YELLOW}No commits found in repository. Skipping commit signoff check.{RESET_COLOR}", ) return PASSOption 1 (silent like others):
- if has_commits() is False: - print( - f"{YELLOW}No commits found in repository. Skipping commit signoff check.{RESET_COLOR}", - ) - return PASS + if has_commits() is False: + return PASS
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
commit_check/author.py(1 hunks)commit_check/branch.py(2 hunks)commit_check/commit.py(3 hunks)commit_check/util.py(0 hunks)tests/util_test.py(2 hunks)
💤 Files with no reviewable changes (1)
- commit_check/util.py
🧰 Additional context used
🧬 Code Graph Analysis (4)
commit_check/author.py (1)
commit_check/util.py (4)
has_commits(33-46)print_error_header(128-144)print_error_message(147-159)print_suggestion(162-173)
commit_check/branch.py (1)
commit_check/util.py (1)
has_commits(33-46)
tests/util_test.py (1)
commit_check/util.py (1)
get_commit_info(48-67)
commit_check/commit.py (1)
commit_check/util.py (1)
has_commits(33-46)
⏰ 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). (2)
- GitHub Check: build
- GitHub Check: Run benchmarks
🔇 Additional comments (8)
commit_check/author.py (2)
4-4: LGTM: Import addition is correct.The
has_commitsfunction is properly imported from the util module and will be used for the early exit check.
8-10: LGTM: Early exit condition handles empty repositories correctly.The check properly returns
PASSwhen there are no commits, which is the expected behavior since author validation is not applicable to empty repositories. The pattern is consistent with similar changes in other modules.commit_check/branch.py (2)
4-4: LGTM: Import addition is correct.The
has_commitsfunction is properly imported from the util module and will be used for the early exit check.
36-38: LGTM: Early exit condition prevents merge base check on empty repositories.The check correctly returns
PASSwhen there are no commits, which is appropriate since merge base validation cannot be performed on empty repositories. This prevents potential errors when running git merge-base commands.tests/util_test.py (2)
126-126: LGTM: Mock simplification improves test readability.Removing the variable assignments for mocked functions that aren't being asserted on later is a good cleanup. The mocks are still being set up properly for the test scenarios.
Also applies to: 144-144, 148-148, 160-160
154-154: LGTM: Expected return value aligns with simplifiedget_commit_infobehavior.The change from
"Repo has no commits yet."to" fake commit message "reflects that theget_commit_infofunction was simplified and no longer handles the no-commits case internally. This is consistent with the PR's approach of handling empty repositories at the calling function level.commit_check/commit.py (2)
5-5: LGTM: Import addition is correct.The
has_commitsfunction is properly imported from the util module and will be used for the early exit checks.
26-28: LGTM: Early exit condition prevents commit message check on empty repositories.The check correctly returns
PASSwhen there are no commits, which is appropriate since commit message validation cannot be performed on empty repositories. This prevents potential errors when trying to read commit messages.
CodSpeed Performance ReportMerging #253 will degrade performances by 21.9%Comparing Summary
Benchmarks breakdown
|
|
closes #251
Summary by CodeRabbit
Bug Fixes
Tests