7.0 KiB
7.0 KiB
id, title, category, status, source_trust_level, verification_status, created_at, updated_at, tags, tech_stack, applied_in, aliases
| id | title | category | status | source_trust_level | verification_status | created_at | updated_at | tags | tech_stack | applied_in | aliases | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| productivity-code-review | Code Review — Checklist / 좋은 review / 빠른 turnaround | Coding | draft | B | conceptual | 2026-05-09 | 2026-05-09 |
|
|
|
Code Review
좋은 review = 학습 + 품질. Severity 라벨 (must-fix / suggestion / nit) + 구체적 + 빠른 turnaround. Conventional comments + 자동 lint + 최소 인간 review.
📖 핵심 개념
- Severity: 변경 강제 vs 권장.
- Specific: 줄 인용 + 예시 코드.
- Fast: 1-2일 안 review.
- Automation: lint / test / type 자동.
💻 코드 패턴
Severity labels
must-fix: Block merge. Bug / 보안 / 명확 잘못.
should-fix: 강 권장. Reviewer 가 확신.
suggestion: Better way 제안.
nit: 취향. Author 가 무시 OK.
question: 이해 / 의도 명확화.
praise: 좋은 코드 — 강화.
Conventional Comments
must-fix: This will throw NPE when user is null.
suggestion: Consider extracting this to a helper.
question: Is this intentional? Looks like a typo.
nit: Maybe rename `x` to `index` for clarity.
praise: Nice use of `early return` here.
→ https://conventionalcomments.org
Inline 예
+ const total = items.reduce((sum, i) => sum + i.price, 0);
must-fix:
i.price가 string 으로 올 수 있음 (API 가 decimal string 반환).const total = items.reduce((sum, i) => sum + Number(i.price), 0);
Checklist (general)
□ Tests pass (CI)
□ Type / lint clean
□ Test coverage 추가됨 (새 logic)
□ Security 검토 (사용자 input, auth, secrets)
□ Performance (N+1, big O)
□ Error handling (네트워크 / DB / null)
□ Logging (의미 있는 + PII 제외)
□ Migration safe (idempotent, rollback)
□ Feature flag 적절
□ Docs / changelog 갱신
Domain-specific
Frontend:
□ A11y (label, keyboard)
□ Loading / error state
□ Bundle size 영향
□ React rendering (memo)
□ i18n
□ Mobile responsive
Backend:
□ Idempotency
□ Rate limit / auth
□ DB index
□ Transaction boundary
□ Backwards-compatible API
DB migration:
□ Concurrent / online
□ Rollback plan
□ Big table 영향
□ Data backfill
□ Lock 시간
Self-review (PR 올리기 전)
1. PR 의 diff 본인 review
2. Description = "what + why"
3. Screenshot / video (UI 변경)
4. Test plan
5. Linked issue / spec
6. Migration / rollback note
→ 본인 review 가 reviewer 시간 절약.
Good PR description
## What
- Add `/api/users/:id/orders` endpoint
- Return paginated list (cursor-based)
## Why
- Customer dashboard needs order history (issue #123)
## How
- Added `OrderRepository.findByUser(userId, cursor)`
- Reused existing `Pagination` helper
- Tests in `orders.test.ts`
## Screenshots
[before/after]
## Migration
None.
## Test plan
- [ ] Unit tests pass
- [ ] Manual: visit `/api/users/u1/orders?limit=20`
- [ ] Manual: pagination cycle
Bad PR
- "fix"
- 1000+ lines
- Multiple unrelated changes
- No description
- No tests
→ Reject + ask to split.
PR size guideline
< 50 lines: < 30 min review
50-200: 1 hour
200-500: half day — split 권장
500+: 분할 strongly
→ 작은 PR = 빠른 review + bug 적음.
Review 속도
Day 1: 첫 review (24h goal)
Day 2-3: Iteration
Day 3-5: Merge
→ Stale PR = context loss.
CODEOWNERS
# .github/CODEOWNERS
/api/payments/ @payments-team
/lib/security/ @security-team
*.tf @platform-team
→ 자동 review 요청.
Review 도구
GitHub PR / GitLab MR / Bitbucket
Reviewable.io
Graphite — stack of small PR
Stack PR (Graphite, etc)
큰 feature = 여러 작은 PR (의존):
1. PR #1: Add interface (small)
2. PR #2: Add impl (depends on #1)
3. PR #3: Wire up (depends on #2)
→ 각 PR 독립 review. 빠름.
Auto review (LLM)
GitHub Copilot / Cursor / Greptile / CodeRabbit / Coderabbit:
- 자동 comment
- Style / common mistake
- 학습 데이터로 quality
→ 인간 보다 먼저 보고 작은 issue 처리.
⚠️ 본질 / 디자인 review = 인간.
Pair programming vs PR
Pair: real-time, 학습.
PR: 비동기, 기록.
→ Hybrid: 큰 / 위험 = pair. 일반 = PR.
Praise (감사)
"Nice refactor — much cleaner."
"Good catch on the edge case."
"Love the test coverage."
→ 강화 + 팀 좋은 분위기.
Disagreement
1. 이해 시도 — "Help me understand why ..."
2. Trade-off 명시 — "I'd prefer X because Y, but I see your point about Z."
3. Author 결정 우선 (own the code).
4. Critical 만 escalate.
→ Personal 안 됨.
Review during oncall
On-call 중 = limited review time.
긴급 만 — 큰 review skip OK.
비-on-call 가 review.
Review SLO
Goal: 80% PR 가 24h 안 첫 review.
큰 PR (500+): 다음 day OK.
→ Stale PR alarm (3+ days no review).
LGTM 의미
"Looks Good To Merge" — 너의 책임으로 merge OK.
모든 거 검증 X. Approve = trust.
모든 comment thread close 후 merge
Approve + 모든 comment resolved → merge.
"resolved" 안 하면 author 가 잊음.
Comment 답
- "Done" — 변경했음.
- "Will do in follow-up #issue" — 별 PR 로.
- "Disagree because ..." — 이유.
- "Won't fix" — 의도적.
Post-merge review (감사)
이미 merge — but 학습.
- "이 부분 다음에 더 작은 PR 가 좋을 듯"
- "Naming 가 더 좋을 수 있음"
→ Author 가 이미 ship — 가르침 / 학습 만.
Review 가 비싸 = 적게 review
Junior PR = 자주 review (학습).
Senior 의 작은 PR = 빠른 LGTM OK.
큰 / 디자인 = 깊이 review.
Test PR before merge
- 직접 checkout + run
- 또는 preview environment (Vercel)
- Edge case manual test
→ "merge → bug" 보다 5분 더 worth.
🤔 의사결정 기준
| 작업 | review depth |
|---|---|
| Hotfix urgent | Quick review (1h) |
| Junior 의 새 feature | Deep review + teach |
| Senior 의 작은 refactor | Quick LGTM |
| 큰 architecture | 면대면 + design doc |
| Production migration | 추가 reviewer + runbook |
❌ 안티패턴
- Severity 무 — 모든 게 must-fix: nit 도 block.
- Description 빈약: reviewer 추측.
- 큰 PR (1000+): 깊이 review 못 함.
- Stale PR (1 week+): context loss.
- Personal attack: 코드 ≠ author.
- Bikeshed (사소): must-fix vs nit 구분.
- Ship 전 self-review 없음: reviewer 시간 낭비.
- Test 없는 PR + LGTM: bug merge.
🤖 LLM 활용 힌트
- Conventional comments + severity.
- Self-review 가 first.
- Auto LLM review = 기본 catch.
- 인간 = 디자인 / 의도.