🤖 Add a policy for writing code comments - #227
Conversation
|
Warning Review limit reached
Next review available in: 5 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
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 |
e18bf73 to
dc9cd4d
Compare
cf185d1 to
36a8fd4
Compare
Every comment in the async teardown migration opened by narrating the code and only then reached its point. Reviewing them one at a time, the prose describing what each comment *should* say was consistently better than the comment itself — which is a repeatable failure, not a one-off. The rule: a comment must say something the code cannot. The test is four questions, and the fourth — why do we need this comment at all — is the one that removes most of them. Examples are the real before/after pairs from that review, including the three kinds of comment worth keeping: an ordering constraint, a deliberate deviation from another policy, and a reach into a private API. Cross-referenced from documentation.md, which governs the outward-facing docs held to the opposite standard.
Violations must be fixed before merge.
36a8fd4 to
7e4bb6c
Compare
commit: |
Stacked on #225 — both add a row to
.policies/index.md, so basing this onmainwould guarantee a conflict. GitHub will retarget tomainautomatically once #225 merges.Why
Every comment I wrote in #226 opened by narrating the code and only then got to the point. Reviewing them one at a time with @taras, the same thing happened five times: the prose describing what the comment should communicate was better than the comment itself.
That's a repeatable failure mode, not a one-off, and nothing in
.policies/currently covers it —documentation.mdgoverns outward-facing docs (JSDoc, examples, changelogs), which are held to the opposite standard.The rule
A comment must say something the code cannot. The test is four questions:
The fourth removes most comments. If the honest answer is "so the reader understands the code", the code should be clearer instead.
Writing rules
Examples
Real before/after pairs from #226 and #224, not invented ones. Including the three kinds of comment the review concluded were worth keeping:
Don't hoist this above the spawns — teardown would hang waiting on closed)We can't use scoped here to prevent losing the halt on effection older than 4.1)intentionally omitted from Effection's public Coroutine because it's considered a private API)And the ones that were deleted: mechanism-without-instruction, when-instead-of-why, and restating
async-teardown.mdat the call site.State: Strict
Violations must be fixed before merge, alongside Package.json Metadata and No Agent Marketing.
Also here
AGENTS.mdgains a line under Coding Standards. Agents write the comments this policy is about and read that section, not.policies/.documentation.mdcross-references it, so the tension between the two is explicit rather than implied.Open question
Whether the "Commenting to explain a confusing name → Rename it" row belongs here or in
naming-consistency.md.Docs only, no version bumps.