j99way99 코드 리뷰
리뷰의 목적은 지적 개수를 늘리는 것이 아니라 머지 여부를 판단할 근거를 만드는 것이다.
절차
- 범위를 확정한다. 무엇 대비 무엇의 diff인가(브랜치 base, PR, 작업 트리). 범위를 모른 채 파일 전체를 읽고 무관한 기존 코드를 지적하지 않는다.
- 변경 의도를 먼저 파악한다. PR 설명, 커밋 메시지, 관련 이슈. 의도를 모르면 "왜 이렇게 했나"를 지적이 아니라 질문으로 남긴다.
- 위험한 것부터 읽는다. 순서: 데이터 변경 → 인증/인가 → 외부 경계(API, 메시지) → 동시성/트랜잭션 → 그 외 로직 → 가독성.
- 각 지적에 실패 시나리오를 붙인다. "이렇게 하면 이런 입력에서 이런 결과가 난다"를 말할 수 없으면 그것은 취향이지 결함이 아니다.
- 위험도로 분류해 보고한다.
위험도 분류
| 등급 | 뜻 | 처리 |
|---|---|---|
| Blocking | 데이터 손상, 보안 결함, 명백한 오동작 | 머지 전 수정 필요 |
| Should fix | 특정 조건에서 깨짐, 운영 중 문제 소지 | 머지 전 수정 권장, 근거 명시 |
| Consider | 구조·중복·단순화 여지 | 판단은 작성자에게 |
| Nit | 스타일, 네이밍 | 개수 제한. 자동 포매터로 해결 가능하면 지적하지 않는다 |
Blocking 이 하나도 없으면 그렇게 명시한다. 억지로 찾아내지 않는다.
공통 점검 항목
정확성
- 경계값(0, 빈 컬렉션, null, 최대치)에서 동작하는가
- 실패 경로에서 부분 변경이 남는가
- 조기 반환·예외로 인해 건너뛰는 정리 작업이 있는가
데이터
- 되돌릴 수 없는 변경(삭제, 덮어쓰기, 마이그레이션)이 포함되어 있는가
- 동시에 두 요청이 들어오면 깨지는가
보안
- 입력이 신뢰 경계를 넘을 때 검증되는가
- 권한 확인이 호출 경로 전체에서 보장되는가 (한 진입점만 막고 다른 경로가 열려 있지 않은가)
- 로그·응답에 비밀값이나 개인정보가 섞이지 않는가
변경 범위
- 요청받지 않은 수정이 섞여 있는가 (리뷰를 어렵게 만든다)
- 기존 동작을 바꾸는데 호출자가 갱신되지 않았는가
테스트
- 이 변경이 깨졌을 때 실패할 테스트가 존재하는가. 없으면 그것을 지적한다.
언어/프레임워크별 체크리스트
- Java/Spring:
references/spring.md
보고 형식
판정: <머지 가능 | 수정 후 가능 | 재작업 필요>
Blocking
- <파일:줄> 한 줄 요약
실패 시나리오: <입력/상태> → <잘못된 결과>
Should fix
- ...
Consider
- ...
같은 지적을 파일마다 반복하지 말고 한 번에 묶는다. 근거 없이 "리팩터링하세요"라고 쓰지 않는다.