코드리뷰 노하우

11번가에서 근무하시는 백명석 팀장님의 코드리뷰 내용을 정리해보았습니다.

코드리뷰를 왜 해야하나?

• 주 목적 : 품질 문제 검수 (버그, 장애)
• 더 나은 코드 품질 : 아키텍처 속성 개선을 위한 코드 개선 (향후 변경 비용 개선)
• 학습 및 지식 전달
  • 대개의 경우 리뷰어들도 리뷰 과정에서 지식을 얻게 된다 (하드스킬, 소프트스킬)
  • 잘하는 사람이 하는걸 보면 동기 부여가 됨
• 상호 책임감 증대
  • 작업 내용을 모든 팀원이 확인을 해줌
  • 내가 하고 있는 일에 관심을 가져다 줌, 팀웍 상승
• 개발 문화 개선
• 설계 개선 제안

좋은 PR 의 예시

PR 시 자동으로 템플릿을 만들어지게 해서 작업 내용을 잘 작성하여 리뷰어의 시간을 아껴주자
(참고 : PR 자동 템플릿 만드는 방법)

코드 리뷰가 어려운 이유

• 코드에 대한 비판을 자신에 대한 비판으로 이해

  • 코드와 자신을 분리해서 생각해야한다.

• 생각을 글로 전달하는 것에 대한 어려움

  • 오해의 소지가 있다. (음성 톤, 표정의 부재)

• 피드백을 조심스럽게 표현해야 한다.

효율적인 PR 방법

• 스타일에 대한 논쟁은 리뷰에서는 시간 낭비다.

  • 잘 만들어진 Google 스타일 가이드를 따르거나
  • 팀 내의 스타일 가이드를 따르거나

• PR을 올릴 때 저자가 먼저 읽어보고 리뷰어들을 위한 설명을 코멘트로 남겨서 리뷰어의 시간을 절약할 수 있게 하라

• 리뷰어에 팀 모두를 포함하라

  • 많은 사람들이 볼 수록 버그를 더 잘 찾아낼 수 있다.
  • 많은 사람들이 본다는 것을 알면 사람들은 더 잘 하려는 경향이 있다. (코드, PR 작성)

• 의미있는 커밋으로 작은 단위로 분리해서 커밋하라

• 짧게 자주. 그러니 PR 사이즈를 작게 해라

효율적인 리뷰 방법

• 리뷰는 즉시 시작하라

  • 코드 리뷰를 높은 우선순위로 잡아라
    • 저자는 리뷰가 종료될 때까지 대기해야한다. 생산성 떨어짐.
  • 리뷰를 바로 시작하면 선순환됨
    • 코드를 읽고 피드백을 줄 때는 시간을 가지고 진행해도 되지만 시작은 바로 해라
  • 리뷰 라운드의 최대 시간은 하루
  • 사람들은 대부분 리뷰할 시간이 없다고 느낀다
    • 리뷰를 하는 것의 문제라기보다는 조직적인 문제임

• 고수준으로 시작하고 저수준으로 내려가라

  • 리뷰 라운드에서 많은 의견을 남길 수록, 저자가 당황할 위험이 커진다.
  • 하나의 라운드에 20~50 개 정도의 의견은 위험의 시작
  • 초기 라운드에서는 고수준 피드백으로 제한
    • 버그, 장애, 성능, 보안 등
  • 고수준의 피드백이 처리된 후에 저수준 이슈를 처리
    • 설계 개선
    • 변수명 변경, 주석을 명확하게 하는 것 등

• 예제 코드 제공에 관대해라

  • 저자를 기분 좋게 하기 위한 방법
    • 리뷰 중에 선물 주기 (코드 예제)
  • 너무 긴 예제는 관대한 것이 아니라 억압적으로 보임
  • 라운드 당 2 ~ 3 개의 코드 예제로 제한
    • 모든 PR 에 예제를 제공하면 저자가 코드를 작성할 수 없다고 생각한다

• 리뷰의 범위를 존중하라

  • PR에 포함되지 않은 라인은 리뷰 범위가 아니다
  • 수정하지 않은 내용에 대해서는 언급하지 않는다.
  • 단, PR 이 둘러싼 코드에 영향을 미칠 때는 언급하자.
    • ex ) Validation 로직을 제거했는데 메서드 명에 Validation 내용이 있는 경우

• 태그를 활용

  • [Nit]
    • 고치면 좋지만 아니어도 그만을 의미
    • 리뷰어는 항상 더 개선할 수 있는 의견을 자유롭게 남길 수 있어야 함.
    • 중요치 않다면 "Nit" 를 태그로 남겨서 저자가 무시할 수 있도록 할 수 있음
    • 교육적인 목적, 지속적으로 기술을 연마하는 것을 돕는 목적
    • 예 nit: null 대신 Optional 을 쓰면 어떨까요?, OCP 준수를 위해 if-else 추가 대신 Strategy 도입은 어떨까요?

• 한두 등급만 코드 레벨을 올리는 것을 목표로 하자

  • D 등급의 PR을 받으면 저자가 C 나 B 등급을 받도록 도와라
    • 리뷰어가 A+ 등급으로 짤 수 있더라도 강제하지 마라. 도와라.
  • 완전하지는 않아도 충분히 좋은 코드가 되도록
  • 승인을 보류하는 유일한 이유
    • 수 차례의 리뷰 라운드 후에도 코드가 F 상태인 경우
      • F : 기능적으로 틀렸거나, 너무 복잡해서 정합성에 확신이 없는 상태

피드백 방법

• 절대 "너"라고 하지 마라. (너는 왜 맨날...)

  • 비판의 대상은 코드여야 한다. 저자가 아님.
  • ~ 하는게 어떨까요?

• 건설적인 피드백을 하라

  • 동료들 간의 코드리뷰는 경쟁을 하는게 아니다. 팀의 생산성을 높이는 것이다.
  • 코드 리뷰를 자신의 코드에 대한 비판이 아닌 학습의 과정으로 인지하면 전체적인 프로젝트 성공에 기여함
  • 건설적인 피드백은 개발자들이 그들의 실수에서 배우고 역량을 증대하도록 동기부여함
    • 맨날 자기가 하던것만 하니까 실수를 안하는 것이다.
  • 건설적인 피드백을 못하겠으면 차라리 참는게 낫다. 상처만 주기 때문.

• 진정한 칭찬을 하라

  • 대부분의 리뷰어가 잘못된 부분에만 집중
  • PR에서 좋은 변경이 있을 때마다
    • "오 이런 API 도 있었나요. 정말 유용하네요~"
    • "이 방법은 생각도 못했네요"
    • "함수를 분리한 것은 좋은 생각이에요. 훨씬 단순해졌네요"
  • 저자가 주니어 혹은 신규 입사자라면 리뷰에 매우 민감하고 방어적일 수 있음
  • 진심어린 칭찬은 리뷰어가 잔인한 감시자가 아니라 도와주려는 팀동료라는 것을 보여서 긴장감을 낮춤

• 피드백은 명령이 아니라 요청으로 표현해라

  • 이 클래스는 별도의 파일로 분리해라 (X)
  • 이 클래스는 별도의 파일로 분리할 수 있을까요? (O)
  • 이 클래스는 너무 커지는 것 같은데 괜찮을까요? (O)

• 의견이 아니라 원칙에 기반하여 피드백해라

  • 저자에게 의견을 줄 때는
  • "제안하는 변경" 과 "변경의 이유" 를 모두 설명하라
    • 이 클래스를 2개로 분리해야 되요 (X)
    • 지금 이 클래스는 파일 다운로드와 파싱의 2가지 책임을 가지고 있어요.
      다운로더와 파서 2개의 클래스로 분리해서 SRP 를 준수하는 것이 어떨까요? (O)

• 반복적인 패턴에 대해서 피드백을 제한하라

  • 저자의 실수가 동일한 패턴임을 식별했다면 모든 경우를 언급하지 마라
  • 동일 패턴에 대해서 2~3개 정도만 언급하라
    • 실제로는 10군데를 고쳐야 하더라도 저자가 꺠우치고 10군데를 고칠 수 있게 만들어라
    • 만약 말해준 부분만 고치더라도 이해하고 그냥 넘어가라

• 교착상태를 적극적으로 처라하라

  • 교착상태로 향하는지 나타내는 표식
    • 토론의 톤이 점차 팽팽해지고 공격적으로 됨
    • 라운드당 코멘트가 줄어들지 않는 경향을 보임
    • 너무 많은 코멘트에 저항이 보임
  • 코드 리뷰의 최악의 결과는 교착상태다
    • 저자는 코멘트 반영을 거부
    • 리뷰어는 저자가 코멘트를 반영하지 않으니 승인 거부
  • 만나서 얘기하라
    • 화상 혹은 만나서 논의 (특히 복잡한 리뷰)
    • 텍스트 기반 의사소통은 상대가 인간이라는 것을 잊게 함
  • 인정이 불가한 경우
    • 저자에게 논의를 팀장이나 테크 리더에게 확대
    • 다른 리뷰어에게 할당
  • 교착상태로 부터 회복
    • 상황을 관리자와 논의하라
    • 휴식을 가져라. 가능하다면 안정될 때까지 PR을 서로 보내지 마라
    • 갈등 해결책을 학습하라
  • 아주 심각하지 않다면 그냥 인정하고 좋은 관계로 동료와의 협업을 지속해라

코드 리뷰를 하는 아주 재밌는 방법

• PR 을 작성한 사람과 짝 프로그래밍 을 하며 어떻게 고치는 게 좋은지 보여주고 Revert

• PR 을 작성한 사람이 스스로 개선할 수 있도록 기회를 주는

  • 20분 짝 프로그래밍 개선 / 2시간 스스로 개선
  • 그래야 스스로 하는 법을 배움

• 결정은 저자가

  • 완벽한 설계가 아니라 당신이 할 수 있는 최고의 설계를 추구
  • 팀 정신을 유지하기 위해 불완전한 해결책을 받아들여라
  • 모든 설계 결함이 항상 실제로 문제가 되지는 않음

• 코드 리뷰의 목적은 비난이 아니라 배움이다

  • 종종 리뷰어들도 배우게 됨

• 리뷰하려는 코드가 그냥 나쁠 때가 있음

  • 저자가 몸이 않좋거나 집안 문제가 있어서 코드를 평소와 다르게 짬

코드 리뷰 효과

• 예상하지 못한 버그를 타 프로젝트에서 발견
• 시간이 지나니 선플이 달리기 시작
  • 코드가 깔끔하네요
  • 이렇게 구현할 수도 있군요. 좋네요
• 많은 사람이 내가 작성한 코드를 본다는 생각에 PR 전에 한번 더 코드를 다듬게 됨
• 잘하는 사람의 코드를 보다보니 좋은 설계, 아키텍처, 클린코드 등에 관심과 열정이 생김

참고

• 우아한테크세미나 - 지속가능한 SW 개발을 위한 코드리뷰