한번 듣고 평생 써먹는 코드 리뷰 노하우 정리

OKKY 1월 세미나에서 백명석님의 “한번 듣고 평생 써먹는 코드 리뷰 노하우”를 듣고 정리한 내용

💬 왜 코드 리뷰를 해야 하나?

우리가 살고 있는 시대

• “Software is eating the world”
• 소프트웨어에 의해 운영되는 제품과 서비스들의 영역이 늘어나고 있다.
• 4차산업

변동성이 매우 심한 시장. 변화의 속도가 점점 더 빨라지고 있고 예측이 불가능함. 점점 더 애자일하게 일해야함

• 소프트웨어가 현재 글로벌을 점유하고 있는 퍼센트: 2020: 5% → 2030: 10%
  • 더 중요한건 나머지 90%.
• IT가 아닌 영역에서 개발자 수 수요/증가 속도가 IT 영역에서보다 가파름
  • ex. 자동차 산업에서 기계공학보다 소프트웨어 공학자가 더 많이 필요함.
• 비즈니스 성공을 위한 개발의 역할
  • 시장: VUCA
  • 비즈니스: 더 빨리 혁신해야함
  • 개발: DRFR
  • 개발 조직의 성능(생산성)이 중요해짐

개발 생산성

• 출시(배포) 회차가 지날수록 개발자는 많아지지만, 생산성은 점점 떨어진다.

좋은 설계없이 빠르게 빠르게 개발만 한다면, 어느 순간부터는 새로운 기능 추가가 어렵고 기존에 쌓아둔 기술부채를 해결하느라 시간을 많이 허비하고 생산성이 떨어진다.

SW 공학의 특성

• 건축공학 = 설계(Design) + 빌드(Build)
  • 설계: 예측하기 어렵고, 급여가 비싸고 창의적인 사람들을 필요
  • 빌드: 좀 더 예측하기 쉬움
• 설계와 빌드가 분리됨
• 빌드 비용이 비쌈(건축 90%)

• 유지보수 비용이 상대적으로 낮음

• 공학 활동의 최종 목적
  • 빌드 할 수 있는 어떤 종류의 재생산 가능한(Reproducible) 문서
• SW 공학의 설계와 빌드
  • 설계 = 완전한 소스 코드
  • SW 빌드 = 컴파일
• 좋은 설계 ~= 클린코드

• SW 엔지니어: 설계를 잘하는 사람 → 코드를 잘 작성하는 사람

  ?? 아키텍쳐를 잘 구성하는 사람인거 아닌가…

• 클린 코드의 중요성
  • SW의 진정한 비용 ~= 유지보수(전체의 80% 이상)
  • 한 번 작성한 코드는 10번 이상 읽음. 작성보다 이해에 10배의 노력 소요
  • 90% 이상의 시간을 어떤 코드를 이해하는데 사용함
• SW 개발의 단순한 진리
  • “The only way to go fast, is to go well” - Robert C. Martin
  • 시간이 흘러도 생산성 저하, 비용 증가를 막을 수 있는 유일한 방법
  • SW 품질에 신중해야
    • SW의 비용과 품질의 관계는 비정상적, 비직관적
    • 향후 변경 비용을 낮춤으로써 익숙한 트레이드오프를 역전시킴
      • “높은 품질의 제품은 비싸다”
    • 개발자의 전문가 정신 ? 비즈니스 혁신 ?

장인정신

• 지식과 경험의 공유만이 전문성을 갖춘 개발자 육성
• 후배들에게 지식과 경험을 공유(도제관계)
• 코드리뷰
  • 개발자가 지금부터 당장 행할 수 있는 공유 활동
  • 배움을 주고 받으며 지속가능한 SW 개발자가 될 수 있는 실천법

목적

• 주목적: 품질 문제 검수(버그, 장애)
• 더 나은 코드 품질: 아키텍처 속성 개선을 위한 코드 개선(향후 변경 비용 개선)
• 학습 및 지식 전달: 코드, 해결책 등과 관련된 지식 공유에 기여
  • 공유(주고 받는 학습)를 통한 역량 증대 및 성장
  • 참여한 모든 사람들의 배움의 기회
  • 대개의 경우 리뷰어들도 리뷰 과정에서 지식을 얻게 됨(하드스킬, 소프트스킬)
  • 동기부여
• 상호 책임감 증대
  • 집단 코드 오너십 및 결속 증대
  • 내가 하고 있는 일에 관심을 가져주는 것
  • 팀에서 일어나는 일 공유. 내 동료는 무엇을 하나? 팀웍
• 개발 문화 개선
• 설계 개선 제안

💬 코드 리뷰의 절차

• 저자(AUthor)
  • 코드 작성, 리뷰 요청
• 리뷰어
  • 코드를 읽고
  • Merge 가능한지 결정
• 변경 내역(Change List, PR)
  • 리뷰 시작 전에 작성
  • 저자가 머지를 원하는 소스 코드에 대한 일련의 변경 (잘한 것, 아쉬운 것, 눈여겨 볼 것)에 대해 기술

코드리뷰에 시간이 많이 드는데, 어디서 시간을 주로 소요해야하는가?

→ 저자(리뷰이)가 고생해서 리뷰어의 시간을 아껴줘야 한다.

💬 왜 코드 리뷰가 어려운가

• 저자
  • 본인 생각이 멋지다고 PR을 보냄
• 리뷰어
  • 왜 멋지지 않은지에 대한 장황한 이유 설명
• 코드에 대한 비판을 자신에 대한 비판으로 오해
• 코드리뷰는
  • 지식/공학적 결정을 공유하는 기회
  • 공유(잘한 것, 아쉬운 것)를 통해 서로의 지식/경험을 나누며 상호 학습을 통한 역량 증대 수단
  • 코드 토의를 개인적 공격으로 받아들이면 물거품
• PR 특성상 생각을 글로 전달하는 것에 대한 어려움
  • 오해의 위험이 크다(음성 톤, 표정의 부재)
• 때문에 피드백을 조심스럽게 표현하는 것이 더 중요하다.

처음 코드 리뷰를 했을 때

• 금요일 오후 svn commit을 조사하며 리뷰
• 불러서 깨기. 불화 / 갈등
• Git의 등장
  • Local branch에 commit 단위 리뷰를 할 수 있도록
  • SNS 댓글 놀이 느낌

💬 기법들

효율적인 PR 방법

• 지루한 작업은 컴퓨터로 처리 (리뷰어가)
  • 코드를 읽는 것은 인지적 부담이 되는 고수준의 집중이 요구되는 작업
    • 컴퓨터가 할 수 있는 일에 이런 노력을 낭비하지 말라
    • 심지어 기계가 더 잘할 수 있는 일에
  • ex. Formatting Tool
    • 공백, 들여쓰기 오류 등
    • 별도의 커밋/PR로 분리. 리뷰 불필요를 기술해서 리뷰를 생략할 수 있도록.
• 스타일 가이드를 통해 스타일 논쟁을 해소(린트 같은 것)
  • 스타일에 대한 논쟁은 리뷰에서 시간 낭비
    • Option1: Adopt an existing style guide
    • Option2: Create your own style guide incrementally
    • Option3: Hybrid
• PR을 올릴 때 주석 달기(저자가)
  • PR을 올릴 때 저자가 먼저 읽어보고
    → 리뷰어들을 위한 설명을 커멘트로 남겨서
    → 리뷰어들의 시간을 절약할 수 있게 하라

• 모두를 포함하라(최대한 많은 사람을)
  • 많은 사람들이 볼 수록 버그를 더 잘 찾아낼 수 있다
  • 많은 사람들이 본다는 것을 알면 대개 더 잘하려는 경향이 있다.
• 의미있는 커밋으로 분리(작업단위 분리)
  • 내가 짰던 코드를 2주 후에 내가 다시 살펴보면 생소하다.
  • 커밋 기록을 잘 관리하는 것도 다시 버전을 돌이켜 볼 때 중요하다.
  • 혼자하는 코드 리뷰나 마찬가지다.

---

효율적인 리뷰 방법

• 리뷰는 즉시 시작해라
  • 코드 리뷰를 높은 우선순위로
    • 저자는 리뷰가 종료될 때까지 대기(Blocked)함
  • 리뷰를 바로 시작하면 선순환됨
    • 코드를 읽고 피드백을 줄 때는 시간을 가지고 진행해도 되지만 시작은 바로 해라
    • 이상적으로는 수분 내에
  • 리뷰 라운드의 최대 시간은 하루
    • 우선순위 높은 업무로 1일 내 불가하다면 다른 리뷰어 지정
    • 월 1회 이상 재지정 해야한다면 속도를 줄여서 건강한 개발 관습(Practices)을 유지 할 수 있어야함
  • Agile pull requests by Mark Seemann
    • if it hurts, do it more often
    • 아침 스탠드 미팅에 익숙
      • 매일 아침 30분, 점심 식사 후 30분을 리뷰를 위해 미리 확보
    • PR에 포함된 변경이 적도록 노력
      • 반나절 정도 작업한 양 정도
      • 모든 팀원들이 하루에 두 번 작은양의 PR을 리뷰할 수 있고 최대 4시간 안에 리뷰가 완료될 것
    • 근본적인 문제는 사람들이 리뷰할 시간이 없다고 느낀다는 것
      • 당신의 ‘개인 기여’로만평가를 받고 있다면 팀을 돕기 위해 수행하는 모든일은 시간낭비처럼 보임
      • 이것은 리뷰를 하는 것의 문제라기보다는 조직적인 문제
  • Pull Requests vs Pair Programming
    • 트레이드오프: Latency or throughput ?
    • 내성적, 사색, 비동기라면 풀리퀘
    • 외향적, 친밀한 개인적 상호작용 이라면 페어
    • 절대 답은 없다. 앙상블 방식도 괜찮다.
• 고수준에서 저수준으로 리뷰할 것
  • 리뷰 라운드에서 많은 의견을 남길수록, 저자가 당황할 위험이 커짐
  • 하나의 라운드에서 20~50개 정도의 의견을 위험의 시작
  • 초기 라운드에서는 고수준 피드백으로 제한
    • 버그, 장애, 성능, 보안 등
    • Extract Method, Composed Method, Invert-if(복잡도) 등
  • 고수준의 피드백이 처리된 후에 저수준 이슈를 처리
    • (선택적인) 설계 개선
    • 변수명 변경, 주석을 명확하게 하는 것 등
• 예제 코드 제공에 관대해라
  • 저자를 기분 좋게 하기 위한 방법
    • 리뷰 중에 선물코드 주기(예제 코드)
  • 너무 긴 예제는 관대한 것이 아니라 억압으로 보인다
  • 라운드당 2~3개의 코드 예제로 제한
    • 모든 PR에 예제를 제공하면 저자가 코드를 작성할 수 없다고 생각한다는 신호
• 리뷰의 범위를 존중하라
  • 자주 보이는 Ainti-Pattern: PR 근처의 코드를 보고 수정 요청
    • Rule of thumb: PR에 포함되지 않은 라인은 리뷰의 범위가 아니다.
      • 유일한 예외는 PR이 둘러싼 코드에 영향을 미칠 때
      • 변경된 라인 바로 위 오타라도 안하는게 좋음
• 태그를 활용하라 (ex: [Nit])
  • ‘고치면 좋지만 아니어도 그만’을 의미
  • 그래도 보통은 고친다 (https://bit.ly/3ugPKFK)
  • 리뷰어는 항상 더 개선할 수 있는 의견을 자유롭게 남길 수 있어야 함.
  • 중요치 않다면 “Nit”를 태그로 남겨서 저자가 무시할 수 있도록 할 수 있다
  • 교육적인 목적이나 개발자들이 지속적으로 기술을 연마하는 것을 돕는 목적
  • Nit ex: null 대신 Optional은 어떨까요?, OCP 준수를 위해 Strategy 도입은 어떨까요?
• 한 번에 1~2등급만 코드 레벨을 올리는 것을 목표로
  • D 등급의 PR을 받으면 저자가 C~B 등급을 받도록 도와라
  • 완전하지는 않아도 충분히 좋은 코드가 되도록
  • F
    • 기능적으로 틀렸거나
    • 너무 복잡해서 정합성에 확신이 없는 상태
  • 승인을 보류하는 유일한 이유
    • 수 차례의 리뷰 라운드 후에도 코드가 F 상태인 경우

---

피드백 방법

• 절대 “너”라고 하지 말 것(”너는 왜 맨날~”)
  • 리뷰의 핵심은 “무엇이 코드를 나아지게 하는가”
  • 누가 잘못된 아이디어를 냈는지가 아니다.
  • 저자의 방어 유발을 최소화하는 방법으로 피드백하자
    • 비판의 대상은 코드지, 저자가 아니다.
    • “너”: 저자의 주의를 코드에서 자신으로 바꾸는 말이다.
  • “너”만 빼도 저자에 대한 판단이 단순한 정정으로 바뀐다.
  • I Message 대화법: 행동→ 결과→감정
  • “~하는 것을 제안합니다.”, “~하는건 어떨까요?”
    • 물어보면 대답한다. 안한다고 대답하는 것도 OK
• 건설적인 피드백을 하라
  • 동료간의 코드 리뷰는 팀의 생산성을 높이는 것이지, 경쟁을 유발하는게 아니다.
  • 코드 리뷰를 자신(의 코드)에 대한 비판이 아니라 학습의 과정으로 인지하면 프로젝트의 성공에 기여할 수 있다.
  • 건설적인 피드백은 개발자들이 실수에서 배우고 역량을 증대하도록 동기를 부여한다.
  • 건설적인 피드백을 못하겠으면 차라리…
• 진정한 칭찬을 해라
  • 대부분의 리뷰어가 잘못된 부분에만 집중한다.
  • 그러나 리뷰는 긍정적 행위 강화를 위한 값진 기회이기도 하다.
  • PR에서 좋은 변경이 있을 때마다 칭찬을 아끼지 마라.
    • “오 이런 API가 있나요? 정말 유용하네요!”
    • “생각도 못했어요! 정말 좋은 해결책이네요!”
  • 저자가 주니어 또는 신규 입사자라면 리뷰에 매우 민감하고 방어적일 수 있다
  • 진심어린 칭찬은 리뷰어가 잔인한 감시자가 아닌 도와주려는 팀원이라는 것을 일깨워준다.
• 피드백은 명령이 아니라 요청을 표현해라
  • 우리는 일상에서 동료에게 명령을 하지 않는다.
    • 명령형(HOW): “12번 테이블에 자리가 비어있습니다. 우리 가족은 저 자리로 걸어가 앉을 것 입니다.”
    • 요청(WHAT): “4명 앉을 자리를 부탁해요” ← 선언형. Tell, Don’t Ask
  • 하지만 리뷰에서는 강압적인 명령이 종종 발견된다.
    • “이 클래스를 별도의 파일로 분리하세요.”
    • → “이 클래스를 별도의 파일로 분리할 수 있을까요?”
    • → “이 클래스는 너무 큰거 같은데 괜찮을까요?”

• 의견이 아니라 원칙에 기반하여 피드백하라
  • 저자에게 의견을 줄 때는 “제안하는 변경”과 “변경의 이유”를 모두 설명할 것
    • (X) 이 클래스를 2개로 분리해야 해요
    • (O) 이 클래스는 2가지의 책임을 가지고 있어요. 2개의 클래스로 분리해서 SRP를 준수하면 어떨까요?
  • SW는 과학인 동시에 예술이다.
    • 항상 원칙에 기반하여 정확히 뭐가 맞고 틀린지 언급 가능한게 아니다.
    • 그냥 단순히 보기 싫거나 직관적이지 않을 수 있다
    • 무엇을 할 수 있을지 객관적으로 설명하라
      • (X) 이 코드는 혼란스럽네요(”너” 대화법)
      • (O) 저는 코드가 이해하기 어려운거 같아요.
• 반복적인 패턴에 대해서 피드백을 제한하라
  • 저자가 동일한 실수를 반복해서 패턴으로 보인다해도 일일히 언급하지마라
  • 동일한 패턴에 대해 2~3개 정도의 예시만 언급하라
  • 그 이상은 저자에게 개별 사례가 아닌 패턴으로 수정을 요구하라

---

교착상태 시

• 교착상태를 적극적으로 처리해라
  • 교착상태로 향하는지 나타내는 표식이 있다
    • 토론의 톤이 점차 팽팽해지고 공격적으로 변한다
    • 라운드당 코멘트가 줄지 않는 경향을 보인다
    • 너무 많은 코멘트에 저항이 보인다
  • 코드리뷰의 최악의 결과는 교착상태(Stalemate)
    • 코멘트를 반영하지 않으니 승인 거부
    • 저자는 코멘트 반영을 거부
  • 만나서 이야기해라
    • 화상 혹은 만나서 논의(특히 복잡한 리뷰)
    • 텍스트 기반 의사소통은 상대가 인간이라는 것을 잊게한다
  • 인정하거나 Escalate하라
    • 교착상태가 길어지면 관계가 나빠진다(퇴사로 이어짐)
    • 그냥 승인하는 비용(Agree to disagree - 갈등 해결책)
    • 저수준 코드를 무심코 승인하면 SW 품질이 낮아질 수 있다
    • 동료와 너무 다퉈서 함께 일하지 않게 된다면 고수준의 품질을 얻을 수 없다.
  • 인정이 불가능한 경우
    • 저자에게 논의를 팀장이나 테크 리더에게 Escalation
    • 다른 리뷰어에게 할당
  • 교착 상태로부터 회복
    • 상황을 관리자와 논의하라
    • 휴식을 가져라. 가능하면 안정될 때까지 서로 PR을 보내지 마라
    • 갈등 해결책을 학습하라
  • 설계 리뷰를 고려하라
    • 굳이 코드 리뷰 때 논의할 내용이 아닌데 지금 하지 않는가?
    • 설계 리뷰가 존재했는가?
  • 아주 심각하지 않다면
    • 그냥 인정하고 좋은 관계로 동료와 협업을 지속해라
    • Agree to disagree

---

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

• PR을 작성한 사람과 짝 프로그래밍을 하며 어떻게 고치는게 좋은지 보여주고, Revert
• PR을 작성한 사람이 스스로 개선할 수 있도록 기회를 주는 방법
  • 페어로 20분만에 끝난 것도 혼자서 하면 2시간이 걸릴 수도 있다.
  • 그러나 이렇게 해야 “스스로 하는 법”을 배운다.
• 결정은 저자(리뷰이)가 한다
  • “완벽한 설계”가 아니라 “저자가 할 수 있는 최고의 설계”를 추구
  • 팀 정신을 유지하기 위해 불완전한 해결책을 받아들여라
  • 모든 설계 결함이 항상 실제로 문제가 되지는 않는다.
• “코드 리뷰의 목적은 비난이 아니라 배움이다” (https://bit.ly/3ohzTa8)
  • 리뷰어도 저자의 코드를 보고 종종 배우게 된다
• 리뷰하려는 코드가 그냥 나쁠 땐 좀 더 안내하는 가르침으로 전환하라
• https://victorrentea.ro/blog/the-best-code-review/

---

코드 리뷰 문화 정착의 어려움 / 극복 방법

• 온/온프라인의 차이
  • 꾸중 vs 토론
• 저자의 노력
  • 리뷰어 n명의 시간을 절약하는 방법이다
  • 효과적인 리뷰도 가능해진다
• 리더의 관심과 의지
  • 가끔, 그러나 매우 자세히
• 코드 비난에 대한 두려움
• 무엇보다 멋져보여야 하고 싶어진다. 코드리뷰 문화 뿐만 아니라 무엇이든지.
  • IDE 단축키 등 소소하고 재밌는 걸 코드리뷰에 슬며시 알려주는 등
• 좌절할 준비를 해라.
  • 내가 원한다고 남에게 영감을 줄 수는 없다.
  • 나는 나 자신의 태도만 제어 가능하다.
  • 영감은 부산물이다.
    • “내가 어떻게 하면 모범이 될 수 있을까?”
    • 특정 행동이나 기술을 채택하도록 하려면 모범이 되어야 한다
    • 혹은 그 기술을 마스터하거나.
  • 그러니 모범이 되어라. 코드리뷰 뿐만 아니라 모든 곳에서
    • 지각 / 업무기여 / 긍정 / 친절
    • 기술에 대한 마스터
    • 키보드로 코딩할 때 사람들이 경악할 만큼 단축키… (…???)
  • 내 행동을 본 몇몇 사람들은 영감을 받게 된다.

코드리뷰의 효과는 바로 나타나지 않는다. 효과가 나타날거라 믿고 꾸준히 투자를 해야한다.

---

코드리뷰의 효과

• 예상하지 못한 버그를 다른 프로젝트에서 발견하기도 한다
• 시간이 지나면 선플이 달리기도
• 많은 사람들이 내가 작성한 코드를 본다는 생각에 한 번 더 코드를 다듬게 됨
• 좋은 설계, 아키텍쳐, 클린코드, TDD 등에 대한 공감대 / 열정 형성