Files
2nd/10_Wiki/Topics/Coding/Productivity_Code_Review.md
T
2026-05-09 21:08:02 +09:00

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
productivity
code-review
vibe-coding
language applicable_to
Process
Engineering
code review
PR review
must-fix
suggestion
nit
conventional comments

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.
  • 인간 = 디자인 / 의도.

🔗 관련 문서