Initial commit
This commit is contained in:
140
skills/code-review/SKILL.md
Normal file
140
skills/code-review/SKILL.md
Normal file
@@ -0,0 +1,140 @@
|
||||
---
|
||||
name: code-review
|
||||
description: Use when receiving code review feedback (especially if unclear or technically questionable), when completing tasks or major features requiring review before proceeding, or before making any completion/success claims. Covers three practices - receiving feedback with technical rigor over performative agreement, requesting reviews via code-reviewer subagent, and verification gates requiring evidence before any status claims. Essential for subagent-driven development, pull requests, and preventing false completion claims.
|
||||
---
|
||||
|
||||
# Code Review
|
||||
|
||||
Guide proper code review practices emphasizing technical rigor, evidence-based claims, and verification over performative responses.
|
||||
|
||||
## Overview
|
||||
|
||||
Code review requires three distinct practices:
|
||||
|
||||
1. **Receiving feedback** - Technical evaluation over performative agreement
|
||||
2. **Requesting reviews** - Systematic review via code-reviewer subagent
|
||||
3. **Verification gates** - Evidence before any completion claims
|
||||
|
||||
Each practice has specific triggers and protocols detailed in reference files.
|
||||
|
||||
## Core Principle
|
||||
|
||||
**Technical correctness over social comfort.** Verify before implementing. Ask before assuming. Evidence before claims.
|
||||
|
||||
## When to Use This Skill
|
||||
|
||||
### Receiving Feedback
|
||||
Trigger when:
|
||||
- Receiving code review comments from any source
|
||||
- Feedback seems unclear or technically questionable
|
||||
- Multiple review items need prioritization
|
||||
- External reviewer lacks full context
|
||||
- Suggestion conflicts with existing decisions
|
||||
|
||||
**Reference:** `references/code-review-reception.md`
|
||||
|
||||
### Requesting Review
|
||||
Trigger when:
|
||||
- Completing tasks in subagent-driven development (after EACH task)
|
||||
- Finishing major features or refactors
|
||||
- Before merging to main branch
|
||||
- Stuck and need fresh perspective
|
||||
- After fixing complex bugs
|
||||
|
||||
**Reference:** `references/requesting-code-review.md`
|
||||
|
||||
### Verification Gates
|
||||
Trigger when:
|
||||
- About to claim tests pass, build succeeds, or work is complete
|
||||
- Before committing, pushing, or creating PRs
|
||||
- Moving to next task
|
||||
- Any statement suggesting success/completion
|
||||
- Expressing satisfaction with work
|
||||
|
||||
**Reference:** `references/verification-before-completion.md`
|
||||
|
||||
## Quick Decision Tree
|
||||
|
||||
```
|
||||
SITUATION?
|
||||
│
|
||||
├─ Received feedback
|
||||
│ ├─ Unclear items? → STOP, ask for clarification first
|
||||
│ ├─ From human partner? → Understand, then implement
|
||||
│ └─ From external reviewer? → Verify technically before implementing
|
||||
│
|
||||
├─ Completed work
|
||||
│ ├─ Major feature/task? → Request code-reviewer subagent review
|
||||
│ └─ Before merge? → Request code-reviewer subagent review
|
||||
│
|
||||
└─ About to claim status
|
||||
├─ Have fresh verification? → State claim WITH evidence
|
||||
└─ No fresh verification? → RUN verification command first
|
||||
```
|
||||
|
||||
## Receiving Feedback Protocol
|
||||
|
||||
### Response Pattern
|
||||
READ → UNDERSTAND → VERIFY → EVALUATE → RESPOND → IMPLEMENT
|
||||
|
||||
### Key Rules
|
||||
- ❌ No performative agreement: "You're absolutely right!", "Great point!", "Thanks for [anything]"
|
||||
- ❌ No implementation before verification
|
||||
- ✅ Restate requirement, ask questions, push back with technical reasoning, or just start working
|
||||
- ✅ If unclear: STOP and ask for clarification on ALL unclear items first
|
||||
- ✅ YAGNI check: grep for usage before implementing suggested "proper" features
|
||||
|
||||
### Source Handling
|
||||
- **Human partner:** Trusted - implement after understanding, no performative agreement
|
||||
- **External reviewers:** Verify technically correct, check for breakage, push back if wrong
|
||||
|
||||
**Full protocol:** `references/code-review-reception.md`
|
||||
|
||||
## Requesting Review Protocol
|
||||
|
||||
### When to Request
|
||||
- After each task in subagent-driven development
|
||||
- After major feature completion
|
||||
- Before merge to main
|
||||
|
||||
### Process
|
||||
1. Get git SHAs: `BASE_SHA=$(git rev-parse HEAD~1)` and `HEAD_SHA=$(git rev-parse HEAD)`
|
||||
2. Dispatch code-reviewer subagent via Task tool with: WHAT_WAS_IMPLEMENTED, PLAN_OR_REQUIREMENTS, BASE_SHA, HEAD_SHA, DESCRIPTION
|
||||
3. Act on feedback: Fix Critical immediately, Important before proceeding, note Minor for later
|
||||
|
||||
**Full protocol:** `references/requesting-code-review.md`
|
||||
|
||||
## Verification Gates Protocol
|
||||
|
||||
### The Iron Law
|
||||
**NO COMPLETION CLAIMS WITHOUT FRESH VERIFICATION EVIDENCE**
|
||||
|
||||
### Gate Function
|
||||
IDENTIFY command → RUN full command → READ output → VERIFY confirms claim → THEN claim
|
||||
|
||||
Skip any step = lying, not verifying
|
||||
|
||||
### Requirements
|
||||
- Tests pass: Test output shows 0 failures
|
||||
- Build succeeds: Build command exit 0
|
||||
- Bug fixed: Test original symptom passes
|
||||
- Requirements met: Line-by-line checklist verified
|
||||
|
||||
### Red Flags - STOP
|
||||
Using "should"/"probably"/"seems to", expressing satisfaction before verification, committing without verification, trusting agent reports, ANY wording implying success without running verification
|
||||
|
||||
**Full protocol:** `references/verification-before-completion.md`
|
||||
|
||||
## Integration with Workflows
|
||||
|
||||
- **Subagent-Driven:** Review after EACH task, verify before moving to next
|
||||
- **Pull Requests:** Verify tests pass, request code-reviewer review before merge
|
||||
- **General:** Apply verification gates before any status claims, push back on invalid feedback
|
||||
|
||||
## Bottom Line
|
||||
|
||||
1. Technical rigor over social performance - No performative agreement
|
||||
2. Systematic review processes - Use code-reviewer subagent
|
||||
3. Evidence before claims - Verification gates always
|
||||
|
||||
Verify. Question. Then implement. Evidence. Then claim.
|
||||
Reference in New Issue
Block a user