docs: recurring review pitfalls for contributors#2040
Conversation
Distills the defect classes that have blocked real PRs into a pre-push checklist: auth gates + negative tests on new endpoints, dual-direction binding on authenticated channels, fail-closed test provisioning, SecretsStore for anything sensitive, first-element-of-collection bugs, limit-encoding mismatches, UTC timestamps, empty catches, refactor parity, SCHEMA vs MIGRATIONS boot-brick rule, idempotent migrations, fix+test in one PR, fork CI verification, fold-every-finding.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis change adds a contributor guide containing 15 numbered rules for recurring security, correctness, schema, refactoring, testing, CI, and pull request review pitfalls. ChangesContributor review guidance
Estimated code review effort: 1 (Trivial) | ~3 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
|
Code Review SummaryStatus: No Issues Found | Recommendation: Merge The incremental diff adds documentation prose only (lines 102-106 of Note: an existing CodeRabbit finding on line 70 ( Files Reviewed (1 file)
Previous Review Summary (commit 1523a90)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 1523a90)Status: No Issues Found | Recommendation: Merge This PR adds a new documentation file ( Files Reviewed (1 file)
Reviewed by hy3:free · Input: 29.5K · Output: 1.1K · Cached: 60.5K |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@docs/contributor-pitfalls.md`:
- Line 70: Update the wrapped PR reference in the contributor pitfalls document
so the line beginning with “#2037” does not parse as a Markdown heading; escape
the hash or move “PR `#2037`” onto the preceding line while preserving the
existing text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 853e5887-bbb2-4b36-8bd4-73cf5ba5451c
📒 Files selected for processing (1)
docs/contributor-pitfalls.md
| When extracting or splitting a component, diff the capability list before and | ||
| after. If something is intentionally dropped (a panel, revision history, a | ||
| share flow), state it in the PR body and file a follow-up issue. Example: PR | ||
| #2037 silently lost the Share/members panel and per-entry revision history. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Escape the hash in the wrapped PR reference.
Line [70] starts with #2037, so Markdown parses it as a heading and triggers MD018. Escape the hash or keep PR #2037`` on the preceding line.
Proposed fix
-#2037 silently lost the Share/members panel and per-entry revision history.
+\`#2037` silently lost the Share/members panel and per-entry revision history.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #2037 silently lost the Share/members panel and per-entry revision history. | |
| \`#2037` silently lost the Share/members panel and per-entry revision history. |
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 70-70: No space after hash on atx style heading
(MD018, no-missing-space-atx)
🤖 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 `@docs/contributor-pitfalls.md` at line 70, Update the wrapped PR reference in
the contributor pitfalls document so the line beginning with “#2037” does not
parse as a Markdown heading; escape the hash or move “PR `#2037`” onto the
preceding line while preserving the existing text.
Source: Linters/SAST tools
1 participant
A pre-push checklist distilled from the defect classes that have actually blocked PRs here, each with the real PR where it bit us. Fifteen items across security/auth, correctness, store/schema, and process.
Intended audience: everyone opening PRs against this repo, including the builder agents. Review gates already enforce all of these; this just makes them checkable before pushing instead of discovered in review.
Source examples cited: #1935 #1944 #1997 #2009 #2023 #2025 #2026 #2027 #2028 #2034 #2036 #2037.
Summary by CodeRabbit