[G1-Sync] Manual knowledge update
This commit is contained in:
@@ -0,0 +1,374 @@
|
||||
---
|
||||
id: quality-code-review-modern
|
||||
title: Code Review Modern — AI-assisted / async / culture
|
||||
category: Coding
|
||||
status: draft
|
||||
source_trust_level: B
|
||||
verification_status: conceptual
|
||||
created_at: 2026-05-09
|
||||
updated_at: 2026-05-09
|
||||
tags: [quality, review, vibe-coding]
|
||||
tech_stack: { language: "process", applicable_to: ["Engineering"] }
|
||||
applied_in: []
|
||||
aliases: [code review, AI review, CodeRabbit, Greptile, async review, review culture, small PR]
|
||||
---
|
||||
|
||||
# Code Review Modern
|
||||
|
||||
> AI-assisted + culture-aware. **CodeRabbit / Greptile (AI), small PR, async-first, blameless**.
|
||||
|
||||
## 📖 핵심 개념
|
||||
- AI 가 first-pass.
|
||||
- Human = critical 만.
|
||||
- Small PR culture.
|
||||
- Async + thread-based.
|
||||
|
||||
## 💻 코드 패턴
|
||||
|
||||
### AI review (CodeRabbit)
|
||||
```yaml
|
||||
# .coderabbit.yaml
|
||||
language: en
|
||||
reviews:
|
||||
profile: chill # or 'assertive'
|
||||
request_changes_workflow: false
|
||||
high_level_summary: true
|
||||
poem: false
|
||||
```
|
||||
|
||||
```
|
||||
PR open → CodeRabbit 가 review.
|
||||
- Bug detect.
|
||||
- Style suggestion.
|
||||
- Test coverage.
|
||||
- Documentation.
|
||||
|
||||
→ Human 가 critical 만 review.
|
||||
```
|
||||
|
||||
### Greptile / Sourcery
|
||||
```
|
||||
- Greptile: codebase-aware (큰 PR 친화).
|
||||
- Sourcery: 매 commit refactor.
|
||||
- Cursor / Copilot: inline.
|
||||
|
||||
→ 매 tool 의 different focus.
|
||||
```
|
||||
|
||||
### Self-review (먼저)
|
||||
```
|
||||
PR open 전:
|
||||
1. Diff 검토.
|
||||
2. Test 실행.
|
||||
3. Self-comment ("이 가 의도?").
|
||||
4. Description 작성.
|
||||
|
||||
→ 매 reviewer 의 시간 절감.
|
||||
```
|
||||
|
||||
### PR description
|
||||
```markdown
|
||||
## Why
|
||||
[motivation]
|
||||
|
||||
## What
|
||||
[change list]
|
||||
|
||||
## Testing
|
||||
- [x] Unit test added.
|
||||
- [x] Manual test.
|
||||
|
||||
## Screenshot / Loom
|
||||
[image]
|
||||
|
||||
## Out of scope
|
||||
[what NOT done]
|
||||
```
|
||||
|
||||
### Small PR culture
|
||||
```
|
||||
< 100 LOC: ideal.
|
||||
100-400: OK.
|
||||
400+: split.
|
||||
|
||||
→ 작은 PR = 빠른 review = 빠른 feedback.
|
||||
```
|
||||
|
||||
### PR size 측정
|
||||
```yaml
|
||||
# GitHub
|
||||
- name: PR size check
|
||||
run: |
|
||||
LOC=$(gh pr diff ${{ github.event.pull_request.number }} | wc -l)
|
||||
if [[ $LOC -gt 1000 ]]; then
|
||||
gh pr comment --body 'Large PR — consider split.'
|
||||
fi
|
||||
```
|
||||
|
||||
### Review SLA
|
||||
```
|
||||
Target:
|
||||
- First review: < 4 hour.
|
||||
- Approve / changes: < 1 day.
|
||||
- Merge: < 2 day.
|
||||
|
||||
→ Long PR = lead time ↑.
|
||||
```
|
||||
|
||||
### Blameless culture
|
||||
```
|
||||
"이 code 가 잘못" — focus on code.
|
||||
"You wrote bad code" — personal.
|
||||
|
||||
→ "이 가 의도?" 보다 "왜 이 approach?" 가 좋음.
|
||||
```
|
||||
|
||||
### Comment 의 levels
|
||||
```
|
||||
Nit: trivial (skippable).
|
||||
Suggestion: optional improvement.
|
||||
Question: clarify.
|
||||
Required: must fix.
|
||||
|
||||
→ 명시적 prefix.
|
||||
```
|
||||
|
||||
```markdown
|
||||
nit: extra space here.
|
||||
suggestion: consider extracting.
|
||||
question: why this approach?
|
||||
required: race condition here.
|
||||
```
|
||||
|
||||
### Conventional Comments
|
||||
```
|
||||
@<level>: <subject>
|
||||
|
||||
praise: nice abstraction!
|
||||
nitpick: extra newline.
|
||||
suggestion: extract to helper.
|
||||
issue: null pointer possible.
|
||||
question: why X?
|
||||
todo: handle error case.
|
||||
chore: rebase main.
|
||||
```
|
||||
|
||||
→ 명시적 + structured.
|
||||
|
||||
### Approval criteria
|
||||
```
|
||||
Don't block on:
|
||||
- Subjective style.
|
||||
- Out of scope.
|
||||
- Future improvement (별 PR).
|
||||
- Opinion (vs author).
|
||||
|
||||
Block on:
|
||||
- Bug.
|
||||
- Security.
|
||||
- Test missing.
|
||||
- Performance regression.
|
||||
```
|
||||
|
||||
### "Approve with comments"
|
||||
```
|
||||
Critical 가 없 가, suggestion 만:
|
||||
- Approve (merge OK).
|
||||
- Comments 가 author 의 discretion.
|
||||
|
||||
→ Author 가 ignore OK.
|
||||
Reviewer 가 trust.
|
||||
```
|
||||
|
||||
### Pair review (paired)
|
||||
```
|
||||
2 reviewer 가 같은 PR.
|
||||
- Different perspective.
|
||||
- 1 가 OK, 1 가 changes.
|
||||
- Critical PR.
|
||||
```
|
||||
|
||||
### Review rotation
|
||||
```
|
||||
Team 의 round-robin:
|
||||
- Avoid 1 사람 burnout.
|
||||
- 매 사람 의 codebase 익숙.
|
||||
- Knowledge spread.
|
||||
```
|
||||
|
||||
→ GitHub team review (auto round-robin).
|
||||
|
||||
### Stale PR
|
||||
```
|
||||
- 7 day no activity = warning.
|
||||
- 14 day = bot reminder.
|
||||
- 30 day = auto-close.
|
||||
|
||||
→ Backlog hygiene.
|
||||
```
|
||||
|
||||
### Pre-commit (catch first)
|
||||
```bash
|
||||
# .husky/pre-commit
|
||||
npm run lint
|
||||
npm run typecheck
|
||||
npm test
|
||||
```
|
||||
|
||||
→ Local catch = review 전.
|
||||
|
||||
### Conventional commit
|
||||
```
|
||||
feat: add OAuth login
|
||||
fix: resolve race condition in cart
|
||||
chore: update dependencies
|
||||
refactor: extract user service
|
||||
test: add integration tests for checkout
|
||||
docs: update README
|
||||
perf: optimize search query
|
||||
```
|
||||
|
||||
→ Auto changelog + semantic versioning.
|
||||
|
||||
### Review templates
|
||||
```markdown
|
||||
### Functional review
|
||||
- [ ] Code does what it claims.
|
||||
- [ ] Edge cases.
|
||||
- [ ] Error handling.
|
||||
- [ ] Performance.
|
||||
- [ ] Security.
|
||||
|
||||
### Code quality
|
||||
- [ ] Readable.
|
||||
- [ ] DRY (no duplicate).
|
||||
- [ ] Tests.
|
||||
- [ ] Comments where needed.
|
||||
```
|
||||
|
||||
### Type of review
|
||||
```
|
||||
Architectural: design, structure.
|
||||
Functional: behavior, correctness.
|
||||
Style: format, naming.
|
||||
Performance: complexity.
|
||||
Security: vuln.
|
||||
|
||||
→ 매 PR 의 focus 따라.
|
||||
```
|
||||
|
||||
### Reviewer fatigue
|
||||
```
|
||||
1 일 5+ PR = quality ↓.
|
||||
|
||||
→ Limit. Pair.
|
||||
```
|
||||
|
||||
### LLM-assisted human review
|
||||
```
|
||||
Cursor / Copilot 가:
|
||||
- Diff summary.
|
||||
- Specific concern (security, perf).
|
||||
- Refactor suggestion.
|
||||
|
||||
→ Human 가 critical 만 + LLM 가 noise.
|
||||
```
|
||||
|
||||
### Code review 의 ROI
|
||||
```
|
||||
Pros:
|
||||
- Bug catch.
|
||||
- Knowledge transfer.
|
||||
- Code quality.
|
||||
- Mentor.
|
||||
|
||||
Cons:
|
||||
- Time (1 hour / day / dev).
|
||||
- Bottleneck.
|
||||
- Conflict.
|
||||
|
||||
→ Process 의 efficiency 가 key.
|
||||
```
|
||||
|
||||
### Pair programming (review alternative)
|
||||
```
|
||||
2 사람 가 real-time:
|
||||
- 매 commit 가 already reviewed.
|
||||
- 빠른 feedback.
|
||||
- 작은 work 의 sweet.
|
||||
|
||||
→ Critical / 어려운 task 만.
|
||||
```
|
||||
|
||||
### Review metric
|
||||
```
|
||||
- Time to first review.
|
||||
- Iterations per PR.
|
||||
- LOC per PR.
|
||||
- Approval rate.
|
||||
- Bug rate (post-merge).
|
||||
|
||||
→ DORA-style metric.
|
||||
```
|
||||
|
||||
→ [[Quality_Engineering_Excellence]].
|
||||
|
||||
### Tools (modern)
|
||||
```
|
||||
- GitHub PR (default).
|
||||
- Reviewable / Pull Reviewers (3rd party).
|
||||
- CodeRabbit / Greptile (AI).
|
||||
- Graphite (stack-based PR).
|
||||
- Sapling (Meta).
|
||||
```
|
||||
|
||||
### Stack-based PR (Graphite)
|
||||
```
|
||||
1 feature = 5 작은 PR (stack).
|
||||
- 매 PR = 작은.
|
||||
- Sequential merge.
|
||||
- Big feature = manageable.
|
||||
```
|
||||
|
||||
→ 큰 feature 의 답.
|
||||
|
||||
### Best practice
|
||||
```
|
||||
1. Self-review 먼저.
|
||||
2. Small PR (<400 LOC).
|
||||
3. PR description 의 why.
|
||||
4. AI assist (CodeRabbit).
|
||||
5. Async + threaded comment.
|
||||
6. Conventional comments.
|
||||
7. Approve with comments OK.
|
||||
8. Don't block on opinion.
|
||||
```
|
||||
|
||||
## 🤔 의사결정 기준
|
||||
| 작업 | 추천 |
|
||||
|---|---|
|
||||
| AI first-pass | CodeRabbit / Greptile |
|
||||
| Small PR | < 400 LOC |
|
||||
| Big feature | Stack (Graphite) |
|
||||
| Critical | Pair review |
|
||||
| Fast feedback | Pair programming |
|
||||
| Async team | Threaded comment |
|
||||
|
||||
## ❌ 안티패턴
|
||||
- **Big PR (1000+)**: slow review.
|
||||
- **No description**: 시간 낭비.
|
||||
- **Block on opinion**: morale.
|
||||
- **Personal attack**: blameless.
|
||||
- **No SLA**: lead time ↑.
|
||||
- **Manual everything**: AI 의 가치.
|
||||
|
||||
## 🤖 LLM 활용 힌트
|
||||
- AI review (CodeRabbit) 가 first-pass.
|
||||
- Small PR + async + blameless.
|
||||
- Conventional comments 가 structured.
|
||||
- DORA metric 의 lead time.
|
||||
|
||||
## 🔗 관련 문서
|
||||
- [[Productivity_Code_Review]]
|
||||
- [[Productivity_PR_Template]]
|
||||
- [[Quality_Code_Ownership_CODEOWNERS]]
|
||||
Reference in New Issue
Block a user