코드 리뷰에서 진짜 봐야 할 것들: 스타일 지적 50개짜리 PR을 받고 깨달은 것

내가 받은 PR 코멘트 중 최악은 50개짜리였다.

입사 초기, 커머스 플랫폼 팀에서 상품 상세 페이지 리팩토링 PR을 올렸다. 주말까지 써서 만든 300줄짜리 변경이었다. 월요일 아침에 출근해서 GitHub 알림을 열었는데 코멘트가 50개 달려 있었다. 가슴이 뛰었다. 이렇게 열심히 봐줬구나. 하나씩 읽기 시작했다.

"여기 세미콜론 빠졌네요." "indent 4칸인데 2칸으로 맞춰주세요." "import 순서 알파벳으로 해주세요." "여기 trailing comma 추가요."

50개 전부 코드 스타일이었다. 비즈니스 로직에 대한 코멘트는 단 하나도 없었다. 내가 새로 도입한 useProductVariant라는 커스텀 훅의 상태 관리 방식, 재고 부족일 때의 edge case 처리, 그리고 기존 장바구니 로직과의 정합성 — 이런 건 아무도 안 봤다. 2주 뒤에 그 edge case에서 버그가 터졌다. 재고가 0인 상품의 옵션을 선택하면 "장바구니에 담겼습니다"가 뜨는 버그. QA가 잡기 전에 실사용자가 CS를 넣었다.

그날 나는 코드 리뷰라는 프로세스에 대해 근본적으로 다시 생각하기 시작했다.

Lint는 사람이 할 일이 아니다#

먼저 명확히 해두자. 코드 스타일을 리뷰에서 잡는 건 리뷰어의 시간 낭비다. ESLint와 Prettier가 존재하는 이유가 있다.

세미콜론, indent, import 순서, trailing comma — 이건 전부 자동화할 수 있다. CI에 lint check를 넣으면 PR 올리기 전에 걸린다. 우리 팀은 그 사건 이후 husky + lint-staged를 도입했고, PR에서 스타일 관련 코멘트는 사라졌다.

사람이 리뷰해야 하는 건 기계가 잡을 수 없는 것이다. 기계는 "이 변수명이 다음 달에 이 코드를 읽을 사람에게 혼란을 줄 수 있다"는 걸 모른다.

네이밍: 가장 어렵고 가장 중요한 것#

프로그래밍에서 제일 어려운 두 가지가 캐시 무효화와 네이밍이라는 말이 있다. 실제로 팀에서 리뷰하다 보면 네이밍 문제가 정말 많다.

내가 실제로 리뷰에서 봤던 예시 하나를 들어보겠다. 결제 팀 동료가 올린 PR에서 이런 함수를 봤다.

tsx

function processData(items: CartItem[]) {
  return items.filter(item => item.stock > 0).map(item => ({
    ...item,
    finalPrice: item.price * (1 - item.discountRate),
  }));
}

processData라니. 이 함수가 뭘 하는지 이름만 보고 알 수 있는 사람이 있을까? 이건 재고가 있는 상품만 걸러내고 할인가를 계산하는 함수다. 그러면 그렇게 이름을 지어야 한다.

tsx

function filterInStockAndCalcDiscountPrice(items: CartItem[]) {
  return items.filter(item => item.stock > 0).map(item => ({
    ...item,
    finalPrice: item.price * (1 - item.discountRate),
  }));
}

이름이 길어졌다고? 좋다. 6개월 뒤에 이 코드를 보는 사람이 함수 본문을 읽지 않아도 뭘 하는지 안다. 그게 좋은 이름이다.

리뷰에서 "이 이름으로는 의도가 드러나지 않는 것 같아요"라는 코멘트 하나가, 세미콜론 코멘트 50개보다 팀에 더 큰 기여를 한다.

추상화 레벨: 이 함수가 너무 많은 일을 하고 있지 않은가#

주문 관리 어드민 페이지를 만들 때였다. 동료가 useOrderManagement라는 훅을 만들었는데, 그 안에 주문 조회, 주문 상태 변경, 환불 처리, 메모 저장, 엑셀 다운로드 기능이 전부 들어가 있었다. 파일 하나가 400줄이었다.

이런 PR이 올라오면 리뷰어가 해야 할 질문은 이거다. "이 훅이 하나의 책임만 갖고 있나?"

답은 명확히 아니었다. useOrderManagement는 적어도 세 개의 훅으로 쪼개져야 했다. 주문 조회는 useOrderQuery, 상태 변경과 환불은 useOrderMutation, 엑셀은 useOrderExport. 각각이 독립적으로 테스트 가능해지고, 다른 페이지에서 재사용할 수 있게 된다.

그런데 이걸 리뷰에서 지적하려면 리뷰어 본인이 코드를 깊이 읽어야 한다. 세미콜론 찾기보다 열 배는 어렵다. 하지만 이게 진짜 리뷰다.

Edge case: 저자가 생각하지 못한 상황#

리뷰에서 내가 가장 가치 있다고 느끼는 순간은, 코드 저자가 생각하지 못한 edge case를 찾아줄 때다.

몇 달 전에 검색 팀에서 올라온 PR을 봤다. 검색 결과를 무한 스크롤로 보여주는 기능이었다. 코드는 깔끔했다. React Query의 useInfiniteQuery를 잘 활용했고, intersection observer로 스크롤 감지도 잘 되어 있었다.

그런데 하나 물어봤다.

"검색 결과가 0건일 때는 어떻게 되나요?"

빈 배열이 오면 hasNextPage가 true인 상태로 남아 있었다. observer가 계속 다음 페이지를 요청하게 된다. API를 무한히 호출하는 거다. 저자는 검색 결과가 항상 있는 시나리오만 테스트했던 것이다.

또 하나.

"네트워크가 느린 상황에서 유저가 빠르게 스크롤하면요?"

이건 race condition 가능성이 있었다. 페이지 2 요청이 아직 안 왔는데 페이지 3 요청이 나가면 순서가 꼬일 수 있다.

이런 질문을 던지려면 코드를 읽는 것만으로는 부족하다. "이 코드가 실제 환경에서 어떤 상황을 만날까"를 상상해야 한다. 그게 리뷰어의 핵심 역할이다.

리뷰 안티패턴 세 가지#

팀에서 몇 년간 코드 리뷰를 하면서 반복적으로 보이는 문제 패턴이 있다.

Nitpick 폭격#

아까 내가 받았던 50개 코멘트 같은 경우다. 리뷰어가 코드의 본질을 보지 않고 표면적인 것만 잡는 패턴. 이게 반복되면 PR을 올리는 사람이 리뷰를 두려워하게 된다. "또 쓸데없는 지적 50개 달리겠지"라는 생각이 들면, PR을 작게 쪼개거나 일찍 올리는 대신 최대한 미루게 된다. 리뷰 문화가 오히려 코드 품질을 떨어뜨리는 역설.

LGTM 도장#

반대쪽 극단이다. PR이 올라오면 코드를 안 보고 "LGTM"을 찍는 사람. 우리 팀에도 있었다. 평균 리뷰 시간이 2분이었다. 500줄짜리 PR을 2분 만에 승인하는 건, 리뷰를 한 게 아니라 프로세스를 통과시킨 거다.

LGTM 도장의 진짜 문제는 리뷰를 요청한 사람의 성장 기회를 뺏는다는 거다. 좋은 리뷰 코멘트는 시니어의 사고방식을 전달하는 가장 직접적인 채널이다. 그걸 "LGTM" 네 글자로 날리는 건 아깝다.

2000줄 PR 한 번에 리뷰하기#

이건 리뷰어의 잘못이 아닐 수도 있다. 애초에 PR이 너무 크면 제대로 된 리뷰가 불가능하다. Microsoft Research의 연구에 따르면 200줄 이상의 변경에서는 리뷰 품질이 급격히 떨어진다. 2000줄이면 리뷰어의 뇌가 포기한다. 무의식적으로 파일 목록을 스크롤하면서 "음 괜찮아 보이네"를 반복하게 된다.

우리 팀이 도입한 것들#

문제를 인식한 다음에 팀에서 몇 가지를 바꿨다. 전부 한 번에 도입한 건 아니고 분기별 회고에서 하나씩 제안해서 실험한 것들이다.

PR 사이즈 제한. 300줄 이상이면 쪼개자는 가이드라인을 정했다. 강제는 아니고 권장이다. "이거 300줄 넘는데 쪼갤 수 있을까요?"라고 물어보는 게 자연스러운 문화가 됐다. 예외도 있다. 대규모 리팩토링이나 자동 생성 코드는 제한에서 뺐다.

코멘트 접두사. 이게 체감 효과가 가장 컸다. 모든 리뷰 코멘트에 접두사를 붙이기로 했다.

[must-fix] — 이건 고쳐야 머지할 수 있다. 버그, 보안 이슈, 성능 문제.
[nit] — 안 고쳐도 된다. 스타일이나 개인 취향.
[question] — 잘 모르겠어서 묻는 것. 비판이 아니라 궁금한 것.

이게 왜 효과가 컸냐면, 리뷰를 받는 사람의 심리적 부담이 확 줄었다. 예전에는 코멘트 하나하나가 "이거 고쳐"처럼 느껴졌는데, [nit]가 붙으면 "참고만 해"라는 신호가 된다. [question]이 붙으면 "나도 배우고 싶어서 물어보는 거야"라는 의미가 된다.

PR 저자도 뭘 먼저 해결해야 하는지 우선순위가 명확해졌다. [must-fix]만 먼저 고치고, [nit]는 다음 PR에서 반영해도 된다.

셀프 리뷰 체크리스트. PR 올리기 전에 저자가 스스로 확인하는 항목 다섯 개를 만들었다. "네이밍이 의도를 드러내는가", "400줄 이상인 파일이 있는가", "에러 케이스를 처리했는가", "새로 추가한 함수에 대한 테스트가 있는가", "PR description에 왜 이 변경이 필요한지 적었는가". 리뷰어가 잡아야 할 것을 저자가 먼저 거르니까, 리뷰 코멘트의 질이 올라갔다.

리뷰는 코드가 아니라 사고를 보는 것이다#

코드 리뷰를 오래 하다 보면 결국 깨닫게 되는 게 있다. 우리가 보는 건 코드가 아니라 그 코드를 만든 사람의 사고 과정이라는 것.

"왜 이 구조를 선택했지?" "이 추상화 레벨이 적절한가?" "다음 사람이 이걸 이해할 수 있을까?"

이런 질문을 던지는 리뷰가, 세미콜론 50개 잡는 리뷰보다 팀을 성장시킨다. 나도 여전히 가끔 사소한 것에 눈이 가지만, 그럴 때마다 그 50개짜리 PR을 떠올린다. 그리고 코멘트를 쓰기 전에 한 번 더 생각한다 — 이 코멘트가 6개월 뒤에도 의미가 있을까.

리뷰어로서 가장 뿌듯한 순간은 내 코멘트 덕분에 프로덕션 버그를 막았을 때가 아니다. 내 코멘트를 읽은 동료가 다음 PR에서 같은 실수를 스스로 잡아냈을 때다. 그게 코드 리뷰의 진짜 목적이 아닐까.