코드 리뷰 원칙과 체크리스트

코드 리뷰는 내가 쓴 코드의 문제를 혼자서는 다 찾을 수 없기 때문에 하는 일이다. 누구의 코드든 다른 사람의 눈을 거치면 사용자에게 닿기 전에 더 많은 버그를 잡을 수 있다. 완벽한 단 하나의 프로세스는 없으니 팀에 맞는 방식을 찾되, 정기적으로 하는 것이 가장 중요하다.

백명석의 「한번 듣고 평생 써먹는 코드 리뷰 노하우」(OKKY 세미나, 2022.01) 강연을 함께 정리했다. 체크리스트는 Ryan McDermott의 code-review-tips를 바탕으로 재구성했다. 팀 정착 부분은 「효과적인 코드 리뷰를 위해서」, 「개발자 Code Review 가이드」를 바탕으로 보강했다.

왜 코드 리뷰를 하나

관찰	의미
출시를 거듭할수록 개발자는 늘어도 생산성은 떨어진다(『클린 아키텍처』 1장)	설계가 망가지면 새 기능보다 기존 코드와 씨름하는 데 시간을 쓴다
설계 체력 가설(Design Stamina Hypothesis)	좋은 설계 없이 빨리 가면 어느 시점부터 누적 기능 속도가 꺾인다. 빨리 가는 유일한 방법은 잘 가는 것이다
소프트웨어는 소스 코드가 곧 설계다	다이어그램만으로는 같은 결과물이 나오지 않는다. 설계를 잘하려면 코드를 잘 써야 한다
유지보수가 전체 비용의 대부분이고, 코드는 쓰는 것보다 읽는 데 훨씬 많은 시간이 든다	읽기 쉬운 코드가 비용을 줄인다
소프트웨어는 품질이 높을수록 오히려 싸다	다른 분야와 달리 품질과 비용의 트레이드오프가 뒤집힌다. 경영진은 장인 정신이 아니라 변경 비용으로 설득한다

그래서 코드 리뷰는 결함을 미리 찾는 것을 넘어 다섯 가지를 얻는 실천이다.

품질: 배포 전에 결함을 잡고, 아키텍처 품질 속성을 개선해 이후 변경 비용을 낮춘다.
학습과 지식 전달: 리뷰는 시니어가 일방적으로 가르치는 자리가 아니라 주고받는 자리다. 리뷰어도 설명하는 법과 공감하는 법을 배운다.
동기 부여: 다른 팀 리뷰까지 열어 두면 잘하는 사람을 보고 따라 하고 싶어진다.
공동 책임과 결속: 리뷰한 사람도 책임을 느끼고(공동 코드 소유), 옆 동료가 무엇을 하는지 알게 된다.
비난이 아니라 학습: 목적은 누가 잘못했는지가 아니라 코드를 낫게 만드는 것이다.

작성자와 리뷰어가 할 일

작성자가 고생해서 리뷰어 여러 명의 시간을 아껴 준다. 그래야 리뷰가 선순환한다.

작성자	리뷰어
빌드·테스트가 통과하는 PR만 올린다. IDE 검사로 기계가 찾는 결함은 미리 없앤다	받으면 바로 시작한다. 한 라운드가 하루를 넘지 않게 한다
PR은 리뷰어가 한 번에 집중해 볼 수 있을 만큼 작게 만든다	고수준에서 저수준으로 내려간다. 요구사항 충족 → 구조·설계 → 세부 구현 → 스타일 순서다. 한 번에 코멘트가 20개를 넘으면 위험 신호다
변경 설명에 무엇을 왜 바꿨는지, 특히 봐 줬으면 하는 곳, 더 나은 방법이 궁금한 곳을 적는다	변경 범위 밖은 지적하지 않는다. 변경 때문에 함께 고쳐야 하는 곳만 말한다
올리기 전에 스스로 먼저 읽고 설명이 필요한 곳에 PR 코멘트를 단다	예시 코드에 관대하되 다 짜 주지 않는다. 코드 조각 하나가 긴 설명보다 의도를 정확히 전한다
리뷰어를 넉넉히 포함한다. 보는 눈이 많을수록 버그를 더 찾고, 작성자도 더 신경 쓴다	사람이 아니라 코드에 대해, 친절하게 말한다. 코멘트 쓰는 요령은 리뷰 코멘트와 PR 소통에 있다

사람이 볼 것과 아닌 것

자동화할 수 있는 것은 사람이 보지 않는다. 포맷, 세미콜론, let/var 같은 논쟁은 formatter와 linter(ESLint, Prettier)에 맡긴다. 리뷰 시간은 기계가 못 보는 것에 쓴다.
설계 논의는 코드를 쓰기 전에 한다. 콘크리트를 부은 뒤에 기초 공사를 논하지 않는다. API·구조에 대한 큰 결정은 PR 이전(설계 문서, 이슈)에서 합의한다.

체크리스트

가독성

볼 것	예
오타	사소하지만 남으면 검색·이해를 방해한다. 친절하게 수정 제안
이름이 의도를 드러내는가	`u(names)` → `namesToUpperCase(names)`
함수가 한 가지 일만 하는가	"활성 고객 판별"과 "이메일 발송"이 한 함수에 섞여 있으면 분리
파일(모듈)이 응집되어 있는가	가짜 이름 생성기 파일 안에 DB 조회 함수가 있으면 분리
공개 함수에 설명이 있는가	라이브러리처럼 쓰일 `export` 함수는 역할·사용법 주석
복잡한 로직에 주석이 있는가	비트 연산처럼 이름만으로 설명되지 않는 부분은 "왜"를 주석으로

// Before: 두 가지 일을 한다
function emailClients(clients) {
  clients.forEach((client) => {
    const record = database.lookup(client)
    if (record.isActive()) email(client)
  })
}
 
// After: 판별과 발송을 나눈다
const isActiveClient = (client) => database.lookup(client).isActive()
function emailActiveClients(clients) {
  clients.filter(isActiveClient).forEach(email)
}

부작용

함수는 가능한 한 순수하게. 전역 변수를 읽고 바꾸는 함수는 다른 코드를 예측할 수 없게 깨뜨린다. 필요한 값은 인자로 받고 결과는 반환한다.
I/O는 실패를 처리하는가. 파일·네트워크 요청이 실패(reject)하면 무엇을 반환하고 사용자에게 무엇을 보여 줄지 정해져 있어야 한다.

// Before: 전역 name을 문자열에서 배열로 바꿔 버린다
let name = 'Ryan McDermott'
function splitIntoFirstAndLastName() {
  name = name.split(' ')
}
 
// After
function splitIntoFirstAndLastName(fullName) {
  return fullName.split(' ')
}

경계 조건

빈 값(null·빈 배열): 데이터가 하나도 없을 때 화면에 무엇을 보여 주는가?
아주 많은 값: 1만 개가 오면? 페이지네이션이나 무한 스크롤이 필요한가?
딱 하나일 때: "1 dollars"처럼 단수·복수 처리가 어긋나지 않는가?
사용자 입력 크기 제한: 메시지 본문이 수 MB라면 그대로 DB에 저장할 것인가?
예상 못한 입력: 금액 자리에 문자열이 오면? 클라이언트 검증만 믿지 말고 서버에서도 검증한다.

router.post('/transfer-money', (req, res) => {
  const amount = Number(req.body.amount)
  if (!Number.isFinite(amount) || amount <= 0) {
    return res.status(400).json({ message: 'Invalid amount' })
  }
  transferMoney(user.id, req.body.to, amount)
})

보안

XSS: 사용자 입력(쿼리 파라미터 포함)을 innerHTML로 그대로 넣지 않는다. textContent를 쓰거나 이스케이프한다. 자세한 내용은 웹 보안.
개인정보(PII) 노출: 주민번호·카드번호 같은 값을 URL 쿼리 파라미터, 로그, 분석 도구로 보내지 않는다. URL은 서버 로그·브라우저 기록·중간자에게 그대로 남는다.
이 목록은 최소한일 뿐이다. 보안 검사는 커밋마다 자동화하고 정기적으로 감사한다.

성능

알맞은 자료구조: 배열을 매번 끝까지 도는 대신 Set·Map으로 바로 찾을 수 있는지 본다.

// Before: O(n) 탐색 + 찾아도 계속 돈다
function isUserMentioned(mentions, user) {
  let mentioned = false
  mentions.forEach((m) => { if (m.user === user) mentioned = true })
  return mentioned
}
 
// After: 조회가 잦다면 Set으로 O(1)
const mentionedUsers = new Set(mentions.map((m) => m.user))
const isUserMentioned = (user) => mentionedUsers.has(user)

중요한 작업은 기록되는가: 오래 걸리거나 비즈니스에 중요한 작업은 앞뒤로 로그·지표를 남겨 소요 시간을 측정한다(개인정보는 빼고).

테스트

새 코드에는 테스트가 있어야 한다. 버그 수정이면 그 버그가 고쳐졌음을 증명하는 테스트를, 기능 추가면 단위 테스트와 다른 부분과 잘 맞물리는지 보는 통합 테스트를.
함수가 하는 모든 일을 테스트하는가? (반환값만 확인하고 콜백 호출은 확인하지 않는 식의 누락)
엣지 케이스: 날짜에 음수·소수·아주 큰 일수를 더하면?

기타

TODO 주석은 추적한다. 이슈 트래커에 등록하고 번호를 함께 적어 두어야 언젠가 정리된다.
커밋 메시지는 변경을 정확히 설명한다. "고침", "하나만 더" 같은 메시지는 나중에 git blame으로 찾아왔을 때 아무 도움이 안 된다. 관련 이슈 번호를 함께 적는다. 규칙은 커밋 컨벤션 참고.
코드가 해야 할 일을 하는가. 너무 당연해서 오히려 잊기 쉽다. 세부 지적에 빠져 요구사항 자체를 확인하지 않는 일이 없도록 한다.

더 읽을거리

Google Engineering Practices: Code Review: 구글의 리뷰 가이드(한국어 요약)
효과적인 코드리뷰를 위한 리뷰어의 자세 (카카오테크)
다른 사람의 코드를 리뷰하는 방법
clean-code-javascript, Airbnb JavaScript Style Guide
『클린 코드』 4장 주석 정리: 주석이 많다고 좋은 코드가 아니다. 이름과 구조로 의도를 드러내고, 그래도 설명이 안 되는 "왜"만 주석으로 남긴다
책: 『클린 코드』, 『리팩터링』, 『클린 아키텍처』, 『오브젝트』, 『함수형 사고』

리뷰를 팀에 정착시키기

상황	방법
리뷰가 밀린다	"괴로운 일일수록 더 자주 한다." 매일 오전·오후 30분씩 리뷰 시간을 팀 일정으로 박아 둔다. 리뷰가 늦는 것은 개인 문제가 아니라 조직이 리뷰를 중요하게 보지 않는 문제다
PR이냐 페어 프로그래밍이냐	PR은 처리량이 좋지만 지연이 길고, 페어 프로그래밍은 지연이 없지만 둘이 묶인다. 내성적이고 비동기에 익숙한 팀은 PR이, 즉각 대화를 선호하는 팀은 페어가 맞다. 글로 설명이 길어지면 만나서 같이 본다
의견이 맞서 교착된다	텍스트로 계속 다투지 말고 화상이나 대면으로 이야기한다. 그래도 안 되면 인정(치명적이지 않으면 넘어간다)하거나 팀장에게 에스컬레이션한다. 오늘의 품질보다 계속 함께 일할 관계가 남는다
완벽한 설계를 요구하고 싶다	완벽이 아니라 지금 할 수 있는 최선을 고른다. 지적한 설계 문제가 실제 문제가 되는지는 측정해 봐야 안다
무엇이 반려 사유인지 매번 다툰다	실패한 테스트, 스타일 위반, PR 규칙(크기·설명) 위반처럼 반려 기준을 미리 합의해 둔다. 필수가 아닌 제안은 `Nit:`을 붙여 작성자가 고르게 한다
「나중에 정리할게요」	나중은 대개 오지 않는다. 리뷰에서 나온 정리는 머지 전에 끝내는 것을 기본으로 하고, 정말 미룰 때는 이슈로 남긴다
리뷰어가 코드를 이해하지 못한다	이해하지 못한 코드는 승인하지 않는다. 진전이 없으면 그 영역을 아는 리뷰어를 찾는다
리뷰가 품질을 높이는지 모르겠다	결함 추이와 함께 배포 빈도, 변경 리드 타임, 변경 실패율, 복구 시간(MTTR)을 본다. 개별 수치보다 추세를 보고, 커밋마다 품질 지표를 자동으로 알려 즉시 고치게 한다
처음 도입한다	리더의 관심과 의지가 필요하다. 바로 효과가 나지 않으니 익숙해질 때까지 꾸준히 투자하고, 좋은 리뷰 사례를 공유하고, 리팩터링·도구 사용을 직접 보여 줘 따라 하고 싶게 만든다

리뷰를 해도 버그는 생긴다. 리뷰는 버그가 없음을 증명하지 못하고, 결함을 찾을 확률을 높일 뿐이다. 리뷰에서 자주 언급되는 기법을 익히려면 『리팩터링』, 『레거시 코드 활용 전략』, 『단위 테스트』가 좋다.

핵심 정리

리뷰는 품질·학습·동기·공동 책임을 얻는 실천이다. 소프트웨어는 품질이 높을수록 변경 비용이 싸다.
작성자가 먼저 읽고 설명해 리뷰어의 시간을 아끼고, 리뷰어는 바로 시작해 하루 안에 끝낸다.
스타일은 도구에, 설계는 PR 전에, 리뷰는 기계가 못 보는 것에.
가독성 → 부작용 → 경계 조건 → 보안 → 성능 → 테스트 순으로 훑는다.
친절하게, 고수준부터, 예시 코드와 함께.

버그 제보