Add precommit hook for comments#12922
Draft
AustinMroz wants to merge 1 commit into
Draft
Conversation
🎭 Playwright: ✅ 1465 passed, 0 failed · 3 flaky📊 Browser Reports
|
Contributor
📝 WalkthroughWalkthroughA new pre-commit script ( ChangesComment-only commit enforcement
Sequence Diagram(s)sequenceDiagram
participant Git as Git commit
participant Hook as .husky/pre-commit
participant LintStaged as lint-staged
participant CheckComments as check-comment-commits.ts
participant GitDiff as git diff --cached
Git->>Hook: trigger pre-commit
Hook->>LintStaged: pnpm exec lint-staged
LintStaged-->>Hook: pass
Hook->>CheckComments: pnpm exec tsx
CheckComments->>GitDiff: run git diff --cached
GitDiff-->>CheckComments: staged diff text
CheckComments->>CheckComments: analyzeStagedDiff (scanLine per hunk)
alt comment-only or code-only commit
CheckComments-->>Hook: exit 0
Hook-->>Git: commit proceeds
else mixed comment + code changes
CheckComments-->>Hook: stderr report + exit 1
Hook-->>Git: commit blocked
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #12922 +/- ##
===========================================
- Coverage 76.63% 62.15% -14.49%
===========================================
Files 1568 1458 -110
Lines 106246 75063 -31183
Branches 32353 21146 -11207
===========================================
- Hits 81424 46652 -34772
- Misses 23945 28068 +4123
+ Partials 877 343 -534
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/check-comment-commits.ts (1)
53-168: 💤 Low valueRegex literal parsing is not handled, but acceptable for this use case.
The scanner doesn't parse regex literals (
/pattern/), so constructs likeconst re = /foo\/bar/would be parsed character-by-character rather than as a single token. However, since the scanner only looks for//and/*comment markers, this doesn't cause false positives in practice unless a regex contains those exact sequences. Given the PR acknowledges this is a "brittle" POC, this limitation is reasonable.The backslash handling at lines 108-112 (outside strings) primarily handles line continuations and incidentally helps with some regex escapes, though it's not semantically correct for all contexts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check-comment-commits.ts` around lines 53 - 168, The scanLine function does not handle regex literal parsing, which means constructs like regex patterns with escaped forward slashes could theoretically cause issues if they contain comment markers. While the reviewer acknowledges this is acceptable for the current POC use case since the scanner only looks for // and /* patterns, consider adding a code comment above the scanLine function or near the backslash handling logic (around the character-by-character iteration) to explicitly document this known limitation and explain why it's acceptable for this scanner's purpose. This will help future maintainers understand the intentional design decision.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@scripts/check-comment-commits.ts`:
- Around line 53-168: The scanLine function does not handle regex literal
parsing, which means constructs like regex patterns with escaped forward slashes
could theoretically cause issues if they contain comment markers. While the
reviewer acknowledges this is acceptable for the current POC use case since the
scanner only looks for // and /* patterns, consider adding a code comment above
the scanLine function or near the backslash handling logic (around the
character-by-character iteration) to explicitly document this known limitation
and explain why it's acceptable for this scanner's purpose. This will help
future maintainers understand the intentional design decision.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 83d1f0b6-76b7-44f0-8f94-050d8c4a3147
📒 Files selected for processing (3)
.husky/pre-commitscripts/check-comment-commits.test.tsscripts/check-comment-commits.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
An early, aggressive POC: Forbids commits being created that contain both code changes and new comments.
I'm not terribly fond of the implementation. It's rather brittle and particularly vulnerable to false positives. It might be better to loosen the restrictions to only
Which should also cut the implementation complexity by ~90%.