docs(quality): add code review standards and process - #405
Conversation
Introduce docs/quality/code-review-standards.md — a system-level code review standard for the kiwi workspace, complementing the existing quality-gates.md. Contents: - Severity tiers (blocker / suggestion / question / nit) and how each is handled. - Rust/kiwi-specific checklist across 8 areas: error handling (panic governance), concurrency & async, storage & Raft, compatibility, security, performance, maintainability, testing. - Review workflow (author self-check -> reviewer -> merge decision) plus reviewer conduct guidelines. - Toolchain gates and a PR checklist quick reference. - Periodic quality-scan mechanism. Core stance: no panic!/unreachable!/unimplemented! on production paths; deserialization returns Err on unknown values; public APIs must not use unimplemented! as a placeholder; unbounded channels must be justified; unsafe needs a SAFETY comment; TODO/FIXME must carry an owner or issue id.
|
Warning Review limit reached
Next review available in: 32 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough新增 Kiwi 代码审查标准文档。文档规定审查意见等级、专项检查项、PR 流程、合并门禁、工具链要求、Checklist 和定期质量扫描规则。 Changes代码审查标准
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Align the severity examples with the document's blocker rules, clarify that clippy::unwrap_used only enforces unwrap(), and classify the mandatory license header as blocking. Constraint: Limit this follow-up to the three review findings on PR #405. Confidence: High; each conflicting rule now has one consistent classification and matches CLAUDE.md, Makefile, and quality-gates.md. Scope-risk: Documentation-only review policy wording; no source, test, build, or CI configuration changes. Tested: git diff --cached --check; six exact content assertions; fenced-code-block balance; staged-path and staged-diff review. Not-tested: No Rust build or runtime test was run because this follow-up only changes review documentation; new remote CI will run after push. Co-authored-by: OmX <omx@oh-my-codex.dev>
![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/quality/code-review-standards.md`:
- Line 60: 更新代码审查标准中的外部输入校验要求，移除对所有客户端输入统一执行 UTF-8 转换或校验的表述；明确仅对命令语义要求文本的字段校验
UTF-8，并对 Redis key、value 及其他二进制字段按长度、协议和命令语义进行校验。
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 548d084c-1a2b-435d-b3e1-05f5d378e78d
📒 Files selected for processing (1)
docs/quality/code-review-standards.md
Scope UTF-8 validation to fields whose command semantics require text, while requiring Redis keys, values, and other binary fields to retain their original bytes and follow protocol-specific bounds. Constraint: Limit this follow-up to the current CodeRabbit finding on PR #405. Confidence: High; Kiwi represents RESP bulk strings as Bytes and only performs UTF-8 conversion for fields with textual or numeric semantics. Scope-risk: One documentation rule; no parser, command, storage, test, build, or CI behavior changes. Tested: git diff --cached --check; four exact content assertions; RESP BulkString and parser source inspection; staged-path review. Not-tested: No Rust build or runtime test was run because this follow-up only corrects documentation; new remote CI will run after push. Co-authored-by: OmX <omx@oh-my-codex.dev>
Resolve PR #404 against main after PRs #402, #403, #405, and #406 while preserving RESP resource budgets, declaration-independent aggregate growth, bounded pipeline admission, and one shared request deadline. Constraint: Preserve the verified PR #404 scope and publish without force-push Tested: Windows resp 80 unit and 20 integration tests; net 35 lib tests; WSL resp and net suites, TCP regressions, strict Clippy; cargo fmt and diff checks Co-authored-by: OmX <omx@oh-my-codex.dev>
1 participant
Summary
Adds
docs/quality/code-review-standards.md— a system-level code review standard and process for the kiwi workspace, complementing the existingdocs/quality/quality-gates.md(which focuses on CI gates). This document was produced earlier in a code-quality audit of the repo and is now being contributed back as a standalone docs PR.What it covers
blocker/suggestion/question/nit, with how each is handled in review.Core stance
panic!/unreachable!/unimplemented!on production paths (clippy already deniesunwrap_used; these are not covered by that lint and must be reviewed manually).Erron unknown discriminants rather than panicking.unimplemented!()as a placeholder.unsaferequires aSAFETYcomment.TODO/FIXMEmust carry an owner or issue id (ownerless TODOs are treated as defects).Verification
cargo check/cargo clippyare not impacted.docs/quality/quality-gates.md.Note for reviewers
This is a documentation-only PR (single new file, 160 lines). It intentionally does not modify
quality-gates.mdor any CI config — reviewers may want to cross-link the two docs or fold parts intoCONTRIBUTING.mdin a follow-up, but that is out of scope here.Summary by CodeRabbit