코드 리뷰 잘 하는 법, 심각도 라벨로 주니어가 멈추지 않게

리뷰 코멘트를 늘렸더니 주니어가 더 오래 멈춰 있었습니다

3줄 요약

  • 리뷰 코멘트 개수를 늘렸더니 주니어가 수정을 시작하기까지 걸리는 시간이 오히려 길어졌습니다.
  • 원인은 코멘트의 양이 아니라 심각도 표시가 없다는 것이었습니다. 구글 엔지니어링 관행 문서는 라벨이 없으면 저자가 모든 코멘트를 필수로 해석한다고 적고 있고, 1996년 심리학 메타분석에서도 피드백 개입의 3분의 1 이상이 성과를 떨어뜨렸습니다.
  • 오늘은 코멘트 앞에 Must · Consider · Nit 세 글자를 붙이는 것부터 하시면 됩니다.

작년 겨울에 팀에 들어온 주니어 한 명이 있었습니다. 테스트 코드를 성실하게 쓰고, 질문도 잘하고, 리뷰에서 지적받은 것을 하나도 빠뜨리지 않고 고쳐 오는 사람이었습니다. 저는 이 사람이 빨리 크길 바랐고, 그래서 리뷰를 더 꼼꼼하게 달았습니다. 한 PR에 코멘트가 스무 개 넘게 붙는 날도 있었습니다.

두 달쯤 지나서 이상한 걸 발견했습니다. PR을 올린 뒤 첫 커밋이 올라오기까지의 간격이 점점 벌어지고 있었습니다. 처음에는 반나절이면 수정이 올라왔는데, 어느새 하루하고 반나절이 걸렸습니다. 코드가 어려워진 것도 아니고, 다른 일이 많아진 것도 아니었습니다. 1:1에서 물어봤더니 이런 대답이 돌아왔습니다. "코멘트가 스무 개 달리면 어디부터 손대야 할지 모르겠어요. 다 고쳐야 하는 건가 싶어서 계속 읽고만 있게 돼요."

제가 늘린 것은 정보였는데 상대가 받은 것은 부담이었습니다. 이 글은 그 뒤로 넉 달 동안 리뷰 방식을 고치면서 겪은 기록입니다. 결론부터 적자면 저는 코멘트 개수를 줄이지 않았습니다. 대신 코멘트마다 심각도 표시를 붙이고, PR 하나에 들어가는 파일 수에 상한을 두고, 해법을 대신 써 주는 습관을 끊었습니다. 이 셋을 하고 나서 첫 수정 커밋까지의 간격이 예전 수준으로 돌아왔고, 회고에서 "어디부터 고쳐야 할지 모르겠다"는 말이 다시 나오지 않았습니다.

1. 코멘트를 늘린 뒤에 실제로 생긴 일

관찰한 것

제가 본 것은 세 가지였습니다. 첫째, 수정 착수까지의 간격이 길어졌습니다. 둘째, 수정 내용이 기계적으로 변했습니다. 예전에는 "이 부분은 이렇게 바꾸는 게 나을 것 같은데 어떻게 생각하세요"라는 되물음이 왔는데, 어느 순간부터 지적한 것만 정확히 고쳐서 다시 올라왔습니다. 셋째, 본인이 먼저 제안하는 리팩터링이 사라졌습니다.

세 번째가 제일 뼈아팠습니다. 리뷰의 목표는 이 PR을 좋게 만드는 것이지만, 두 번째 목표는 시간이 지날수록 리뷰가 덜 필요해지도록 만드는 것입니다. 구글의 코드 리뷰 코멘트 작성 문서도 같은 순서로 적어 두었습니다. 저는 첫 번째 목표만 붙들고 두 번째를 갉아먹고 있었습니다.

처음에 잘못 세운 가설

제 첫 가설은 "코멘트가 너무 많다"였습니다. 그래서 한 주 동안 의식적으로 절반만 달아 봤습니다. 결과는 애매했습니다. 수정은 조금 빨라졌지만, 이번에는 제가 넘긴 문제가 그대로 머지되어 나중에 회귀 테스트에서 잡히는 일이 두 번 있었습니다. 개수를 줄이는 방향은 품질을 담보로 잡는 거래였습니다.

두 번째 가설이 맞았습니다. 문제는 개수가 아니라 개수를 해석할 방법이 없다는 것이었습니다. 스무 개의 코멘트가 전부 같은 무게로 보이면, 받는 사람 입장에서는 스무 개짜리 필수 과제 목록입니다. 리뷰어인 저는 그 스무 개를 쓰면서 머릿속으로 이미 무게를 나눠 두었습니다. 이건 꼭 고쳐야 하고, 이건 그냥 눈에 걸려서 적었고, 이건 나중에 참고하라고 남긴 것이라고요. 그런데 그 분류는 제 머릿속에만 있었고 화면에는 스무 줄의 회색 글자만 남았습니다.

돌이켜 보면 저는 리뷰를 쓰는 사람의 비용만 계산하고 있었습니다. 코멘트를 하나 더 다는 데 드는 제 비용은 30초입니다. 그런데 그 코멘트를 받은 사람이 "이게 필수인가 아닌가"를 판단하는 데 드는 비용은 30초가 아닙니다. 애매하면 물어봐야 하고, 물어보기 애매하면 그냥 다 고칩니다. 제 30초가 상대의 반나절로 바뀌는 구조였습니다.

2. 함정 네 가지에 이름을 붙였습니다

원인을 찾으면서 제가 밟았던 것들에 이름을 붙였습니다. 이름이 있으면 회고에서 지목하기 쉬워집니다.

함정 이름 증상 왜 생기는가 대응
전부 필수 함정 사소한 제안까지 필수 과제로 읽힘 심각도 표시가 없음 코멘트 앞에 라벨을 붙임
자아 조준 함정 방어적인 답글, 설명이 길어짐 코멘트가 코드가 아니라 사람을 가리킴 주어를 코드로 바꿔 씀
덩치 함정 코멘트는 많은데 쓸모 있는 게 적음 PR 하나에 파일이 너무 많음 파일 수 상한과 분할 규칙
정답 배달 함정 지적한 것만 정확히 고쳐 옴 리뷰어가 해법 코드를 대신 써 줌 문제만 지적하고 판단은 넘김

전부 필수 함정

구글 문서는 심각도 라벨을 권하면서 이유를 한 줄로 적어 두었습니다. 라벨이 없으면 저자는 정보 전달이나 선택 사항으로 쓴 코멘트까지 모두 필수로 해석할 수 있다는 것입니다. 저는 이 문장을 읽고 나서야 제 리뷰 화면을 상대 눈으로 다시 봤습니다. 변수 이름 제안과 널 체크 누락 지적이 화면에서 똑같이 생겼습니다.

자아 조준 함정

같은 문서는 코멘트를 코드에 관해 쓰고 개발자에 관해 쓰지 말라고 합니다. "왜 여기서 스레드를 쓰셨어요"와 "이 동시성 모델은 성능 이득 없이 복잡도만 올립니다"는 담고 있는 정보가 같은데 받는 사람의 반응이 다릅니다.

이 차이를 설명하는 근거가 심리학 쪽에 있습니다. 클루거와 드니시가 1996년 『Psychological Bulletin』에 낸 피드백 개입 메타분석은 607개의 효과크기와 23,663개의 관측을 모아, 피드백이 평균적으로는 성과를 올리지만(d = .41) 3분의 1이 넘는 경우에는 오히려 성과를 떨어뜨린다고 보고했습니다. 두 사람이 제안한 설명은 이렇습니다. 피드백은 주의가 향하는 지점을 바꾸는데, 그 지점이 과제에서 멀어지고 자기 자신에 가까워질수록 효과가 떨어진다는 것입니다.

제 해석으로는 "왜 여기서 스레드를 쓰셨어요"가 정확히 그 일을 합니다. 읽는 사람의 주의를 코드에서 떼어 내 자기 판단력으로 옮깁니다.

덩치 함정

보수, 그라일러, 버드가 2015년 MSR에서 발표한 마이크로소프트 코드 리뷰 연구는 사내 다섯 개 프로젝트의 리뷰 코멘트 150만 건을 분류했습니다. 여기서 나온 관찰 두 가지가 제 상황과 맞았습니다. 하나는 한 변경에 포함된 파일이 많을수록 저자에게 쓸모 있는 코멘트의 비율이 떨어진다는 것이고, 다른 하나는 리뷰어가 쓸모 있는 코멘트를 다는 비율이 입사 첫 해에 크게 오르다가 그 뒤로는 평평해진다는 것입니다.

앞의 관찰이 저를 찔렀습니다. 저는 큰 PR을 받으면 코멘트를 더 많이 달았는데, 그게 정확히 쓸모 비율이 가장 낮아지는 조건이었습니다.

정답 배달 함정

이건 제 습관이었습니다. 문제를 지적한 김에 고친 코드까지 써서 붙여 주면 왕복이 한 번 줄어드니까요. 구글 문서는 이 지점을 분명히 적어 두었습니다. PR을 고치는 것은 리뷰어가 아니라 저자의 책임이고, 문제만 짚고 판단을 저자에게 넘기는 편이 배움에도 낫고 결과물도 나은 경우가 많다는 것입니다. 코드에 더 가까이 있는 사람은 저자이기 때문입니다.

3. 심각도 라벨부터 고쳤습니다

가장 먼저 한 일은 라벨 세 개를 정하는 것이었습니다. 구글 문서는 Nit · Optional(또는 Consider) · FYI를 예로 듭니다. 저희는 여기에 필수를 뜻하는 라벨을 하나 더해 네 개로 쓰다가, 두 달 만에 세 개로 줄였습니다. FYI와 Nit을 사람들이 구분하지 못했기 때문입니다.

라벨 뜻 저자가 할 일 머지 차단
Must 고치지 않으면 머지할 수 없음 반드시 반영 예
Consider 더 나을 것 같지만 판단은 저자 몫 반영하거나 이유를 한 줄로 답글 아니오
Nit 취향·사소한 정리 무시해도 됨 아니오

규약 문안은 팀 위키에 이대로 붙여 두었습니다.

[리뷰 코멘트 심각도 규약]

Must     — 버그, 데이터 손상 위험, 보안 문제, 테스트 없는 신규 분기
           머지를 막습니다. 근거를 반드시 함께 씁니다.
Consider — 설계·가독성 제안. 저자가 반영하지 않아도 됩니다.
           다만 반영하지 않을 때는 이유를 한 줄 답글로 남깁니다.
Nit      — 이름, 줄바꿈, 주석 문구 같은 취향 영역.
           답글도 필요 없습니다. 무시하고 머지해도 됩니다.

라벨이 없는 코멘트는 Consider로 읽습니다.
Must 비율이 전체 코멘트의 30%를 넘으면 PR을 쪼개자는 신호로 봅니다.

마지막 두 줄이 실제로는 가장 자주 쓰였습니다. 라벨을 빼먹었을 때의 기본값을 정해 두지 않으면 예전 상태로 돌아갑니다.

네 개에서 세 개로 줄인 이유

처음에는 구글 예시를 그대로 따라 FYI를 포함해 네 개로 시작했습니다. 두 달 뒤에 뺐습니다. 이유는 단순합니다. 사람들이 FYI와 Nit을 구분하지 못했습니다. 둘 다 "안 고쳐도 되는 것"으로 읽혔고, 그러면 라벨이 두 개일 이유가 없습니다.

라벨 개수를 정할 때 저는 이 기준을 씁니다. 라벨을 하나 더 만들려면, 그 라벨을 받았을 때 저자가 하는 행동이 기존 라벨과 달라야 합니다. Consider는 답글을 남겨야 하고 Nit은 무시해도 되니 행동이 다릅니다. FYI는 Nit과 행동이 같았습니다. 그래서 뺐습니다.

규약을 줄이는 결정이 늘리는 결정보다 어려웠습니다. 이미 위키에 적어 두었고, 두 달 동안 쓴 흔적이 PR에 남아 있으니까요. 그래도 안 쓰이는 규칙을 남겨 두면 나머지 규칙까지 장식처럼 보이기 시작합니다.

코멘트를 다시 쓰는 연습

라벨만 붙인다고 자아 조준 함정이 사라지지는 않았습니다. 팀 세션에서 실제 코멘트 몇 개를 놓고 다시 쓰는 연습을 했습니다.

고치기 전 고친 뒤
여기 왜 try-except로 감싸셨어요? Must 이 except가 AssertionError까지 삼켜서 테스트 실패가 통과로 보고됩니다. 예외 타입을 좁혀 주세요.
이거 네이밍이 좀... Nit data2보다 filtered_orders가 읽기 쉬울 것 같습니다.
이렇게 하는 게 맞아요 (수정 코드 첨부) Consider 이 픽스처가 워커마다 다시 뜨는 구조로 보입니다. 스코프를 어떻게 잡을지 한 번 봐 주시겠어요?

세 번째 줄이 제일 어려웠습니다. 답을 알고 있는데 질문으로 바꿔서 던지는 일은 리뷰어 입장에서 손해처럼 느껴집니다. 왕복이 한 번 더 생기니까요. 저는 이걸 "이번 PR에서 30분을 더 쓰고 다음 PR에서 코멘트 다섯 개를 아끼는 거래"라고 스스로 설명하고 넘어갔습니다.

4. PR 크기와 되물음 규칙을 같이 손봤습니다

라벨은 증상을 다루고, PR 크기는 원인을 다룹니다. 저희는 파일 수 상한을 열 개로 잡고, 넘으면 자동으로 라벨이 붙게 했습니다. 막지는 않고 표시만 합니다.

# .github/workflows/pr-size.yml
# PR에 담긴 파일 수를 세어 라벨을 붙입니다. 머지를 막지는 않습니다.
name: pr-size
on: [pull_request]
jobs:
  size:
    runs-on: ubuntu-latest
    steps:
      - name: 변경 파일 수 계산 후 라벨 부여
        env:
          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
          PR: ${{ github.event.pull_request.number }}
          REPO: ${{ github.repository }}
        run: |
          # changed_files 는 PR 메타데이터에서 바로 읽을 수 있습니다
          n=$(gh pr view "$PR" --repo "$REPO" --json changedFiles -q .changedFiles)
          echo "변경 파일 수: $n"
          if [ "$n" -gt 10 ]; then
            gh pr edit "$PR" --repo "$REPO" --add-label "size/large"
            gh pr comment "$PR" --repo "$REPO" \
              --body "파일 $n개입니다. 리뷰 쓸모 비율이 떨어지는 구간이라 분할을 권합니다."
          fi

숫자 열 개에 근거가 있는 것은 아닙니다. 저희 저장소에서 리뷰가 하루 안에 끝난 PR들의 중앙값 근처를 골랐을 뿐이고, 팀마다 다시 재야 합니다. 다만 상한이 있다는 사실 자체가 효과가 있었습니다. 상한을 넘길 것 같으면 사람들이 알아서 쪼개기 시작했습니다.

1:1에서 쓰는 확인 질문

라벨 규약을 도입하고 한 달쯤 뒤에 1:1에서 이 질문들을 돌렸습니다. 리뷰가 실제로 어떻게 읽히는지는 본인에게 물어보는 것 말고 알 방법이 없었습니다.

- 지난 PR에서 `Must`가 아닌데 `Must`처럼 읽힌 코멘트가 있었나요?
- 반영하지 않고 넘어간 `Consider`가 있었나요? 없었다면 왜였을까요?
- 리뷰 코멘트를 읽고 나서 손대기까지 가장 오래 걸린 건 어떤 코멘트였나요?
- 제가 답을 써 주는 편이 나았던 순간과 아니었던 순간을 하나씩 꼽아 주세요.

두 번째 질문이 유용했습니다. Consider를 하나도 안 넘긴 사람은 여전히 전부 필수로 읽고 있다는 뜻이었고, 그 사람에게는 규약을 한 번 더 설명해야 했습니다.

왜 라벨만으로는 부족한가

에이미 에드먼드슨이 1999년 『Administrative Science Quarterly』에 낸 팀 심리적 안전감 연구는 제조업 51개 팀을 조사해, 팀의 심리적 안전감이 학습 행동과 연결되고 학습 행동이 안전감과 성과 사이를 매개한다고 보고했습니다. 라벨은 표기 규칙이고, Consider를 실제로 넘기려면 넘겨도 괜찮다는 믿음이 필요합니다. 저는 제가 단 Consider를 저자가 거절했을 때 그 답글에 좋아요를 누르고 넘어가는 것으로 그 믿음을 만들었습니다. 말로 설명하는 것보다 그게 빨랐습니다.

5. 도입 난이도와 유지 비용

항목 심각도 라벨 PR 파일 수 상한 해법 대신 질문
도입 난이도 낮음. 위키 한 페이지와 합의 한 번 중간. 워크플로 파일 하나와 임계값 합의 높음. 리뷰어 습관을 바꿔야 함
유지 비용 낮음. 라벨 빠진 코멘트를 지적하는 정도 낮음. 임계값을 분기마다 다시 봄 중간. 급할 때 제일 먼저 무너짐
학습곡선 하루. 라벨 세 개 외우면 끝 하루 두세 달. 리뷰어마다 편차가 큼
효과가 보이기까지 1~2주 2~4주 한 분기

표를 만들면서 알게 된 것은 세 가지가 서로 다른 층에 있다는 점이었습니다. 라벨은 표기의 문제라 합의만 하면 끝나고, PR 크기는 도구의 문제라 워크플로 하나로 강제할 수 있습니다. 그런데 해법 대신 질문을 던지는 것은 사람의 문제라 강제할 방법이 없습니다. 저는 이걸 리뷰어끼리의 상호 지적으로 처리했습니다. 서로의 리뷰에서 해법 코드가 통째로 붙어 있는 걸 보면 "이건 질문으로 바꿔도 될 것 같은데요"라고 답글을 답니다. 처음에는 어색했는데 두어 달 지나니 자연스러워졌습니다.

셋 중 하나만 고른다면 심각도 라벨입니다. 비용이 가장 싸고 효과가 가장 빨리 보입니다. 해법 대신 질문은 가장 오래 걸리고, 릴리스가 급한 주에는 저부터 무너졌습니다. 저는 이걸 실패로 세지 않고, 급한 주에는 대놓고 "이번 주는 제가 답을 씁니다"라고 말하고 넘어가는 방식으로 처리했습니다.

6. 권할 만한 경우와 권하지 않는 경우

상황 권함 이유
주니어가 팀에 새로 들어온 경우 예 코멘트를 해석하는 기준이 없는 시기입니다
리뷰어가 3명 이상이고 스타일이 제각각인 경우 예 표기가 통일되면 저자의 부담이 줄어듭니다
리뷰 코멘트가 PR당 열 개를 넘는 경우 예 정확히 이 글이 다룬 상황입니다
2인 팀이고 서로 맥락을 다 아는 경우 아니오 라벨 없이도 무게가 전달됩니다. 규약만 늘어납니다
리뷰 자체가 형식적으로만 돌고 있는 경우 아니오 먼저 리뷰가 실제로 이뤄지게 만드는 편이 순서입니다
심리적 안전감이 낮아 반박이 안 나오는 팀 아니오 Consider를 아무도 거절하지 않아 라벨이 무의미해집니다

마지막 줄이 중요합니다. 라벨은 이미 말이 오가는 팀에서 말의 무게를 구분해 주는 도구이지, 말이 없는 팀을 말하게 만드는 도구가 아닙니다.

이번 주에 해볼 수 있는 체크리스트

□ 최근 머지된 PR 3개를 열어 코멘트를 Must / Consider / Nit으로 사후 분류해 본다
□ Must 비율이 30%를 넘는 PR이 있으면 그 PR의 파일 수를 센다
□ 라벨 규약 문안을 위키 한 페이지로 만들고 기본값(라벨 없음 = Consider)을 명시한다
□ 이번 주 리뷰에서 라벨을 실제로 붙여 본다. 빠뜨려도 지적하지 않는다
□ 다음 1:1에서 "Must가 아닌데 Must처럼 읽힌 코멘트가 있었나요"를 묻는다
□ 한 달 뒤, 저자가 거절한 Consider가 몇 건인지 센다. 0건이면 규약이 아직 안 읽힌 것이다

댓글

이 블로그의 인기 게시물

옵시디언 3기기 동기화, 유료 결제 없이 구글 드라이브로 끝내기

최저가 주유소, 얼마나 돌아가면 손해일까?

회의시간 줄이는 법, 개수보다 배치 먼저

유류세 인하 2개월 연장, 주유소 가격 실제 인하폭 계산

국민연금 추납 조회, 미루면 얼마나 더 낼까?