주제 : 지속가능한 SW 개발을 위한 코드리뷰
세미나 보러가기 링크

왜 코드 리뷰를 해야 하나?

일반적인 공학

• 공학 = 설계 + 빌드
• 설계 : 예측하기 어렵고, 급여가 비싸고 창의적인 사람들 필요
• 빌드 : 좀 더 예측하기 쉬움
• 설계가 어려움, 유지보수가 쉬움

소프트웨어 공학

• 공학 활동의 최종 목적

  • 재생산 가능한 문서
• 설계 = 완전한 소스 코드, 좋은 코드 → 비용이 비쌈, 중요함
• SW 빌드 = 컴파일 → 비용이 저렴함
• 좋은 설계 = 클린코드
• SW엔지니어 : 설계를 잘하는 사람 → 코드를 잘 작성하는 사람

클린 코드의 중요성

• SW의 진정한 비용 = 유지보수(전체의 80%)
• 한번 작성한 코드는 최소 10번 이상 읽음, 작성보다 이해에 10배의 노력 소요
• 90% 이상의 시간을 코드를 이해하는데 사용

SW 개발의 단순한 진리

• 시간이 흘러도 생산성 저하, 비용 증가를 막을 수 있는 방법 → 좋은 코드를 작성

• SW 품질에 신중해야 함

  • SW의 비용과 품질의 관계는 비정상적, 비직관적 → 비용이 비쌀 수록 품질이 좋은 것은 아님
  • 향후 변경 비용을 낮춤으로써 높은 품질의 제품은 비싸다는 트레이드오프를 역전
• 테스트코드가 있어야 변화에 적은 비용으로 빠르게 대처할 수 있다.

애자일이 실패하는 이유?

• 스크럼과 같은 프로세스만 강조, 따라하기 때문에
• XP적 요소(TDD, 리팩토링, 자동화 테스트, 단순화된 설계)가 필요함
• 공유 정신이 필요

코드 리뷰

• 개발자가 지금부터 당장 행할 수 있는 공유 활동
• 배움을 주고 받으며 지속가능한 SW 개발자가 될 수 있는 실천법
• Peer Review, Pull Request, Merge Review

목적

• 주목적 : 품질 문제 검수
• 더 나은 코드 품질 : 아키텍처 속성 개선을 위한 코드 개선(향후 변경 비용 개산)

• 학습 및 지식 전달

  • 공유(주고 받는 학습)를 통한 역량 증대 및 성장
  • 하드스킬, 소프트 스킬을 배울 수 있음
  • 시니어도 코드 리뷰를 통해 소프트 스킬(커뮤니케이션, 리더십 등)을 배울 수 있음
• 동기부여

• 상호 책임감 증대

  • 집단 코드 오너십 및 결속 증대
  • 팀에서 일어나는 일 공유
  • 팀웍을 만들어낼 수 있음

• 설계 개선 제안

  • 좋은 사례 공유, 부족한 부분에 대한 질문
• 개발 문화 개선

코드 리뷰의 절차

• 저자 : 코드 작성, 리뷰 요청, PR 요청
• 리뷰어 : 코드를 읽고 머지 가능한지 결정, 글로 피드백

• 좋은 예시

  • Description : PR에 대한 설명
  • How to Test : 어떤 명령을 통해 테스트를 할 수 있는지
  • Commits : 커밋 내역

  → 저자가 고생해서 리뷰의 시간을 아껴줘야 함

코드 리뷰가 어려운 이유

• 저자는 본인 생각에 멋지다고 생각하는 PR을 보냄
• 리뷰어는 왜 멋지지 않은지에 대한 장황한 이유를 작성
• 코드에 대한 비판을 자신에 대한 비판으로 이해
• 코드 리뷰는 지식, 공학적 결정을 공유하는 기회
• 공유를 통해 서로의 지식, 경험을 나누며 상호 학습을 통한 역량 증대 수단
• 코드 토의를 개인적 공격으로 받아들이면 안됨

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

  • 모든 것을 모른다고 가정하고 자신의 생각을 정리해서 말해야 한다.
  • 오해의 위험이 큼(음성 톤, 표정의 부재)
• 피드백을 조심스럽게 표현하는 것이 더 중요

기법들

• 효율적인 PR 방법
• 효율적인 리뷰 방법
• 피드백 방법
• 교착상태 시(싸울 경우)
• 추가 사례

효율적인 PR 방법

• 지루한 작업은 컴퓨터로 처리

  • 코드를 읽는 것은 인지적 부담이 되는 고수준의 집중이 요구되는 작업

    • 컴퓨터가 할 수 있는 일에 노력을 낭비하지 말자
    • 심지어 기계가 더 잘 할 수 있는 일도 있다.

• 스타일 가이드를 통해 스타일 논쟁을 해소

  • Formatting Tool

    • 공백, 들여쓰기 오류 등
  • 좋은 스타일 가이드를 가져다 씀
  • 점진적으로 자신들만의 스타일 가이드를 발전시킴
  • 위 두가지를 합쳐서 사용

  • unused import, declaration

    • 코드 리뷰에 unused import나 declaration에 대한 리뷰가 종종 있는데 IntelliJ inspection을 error로 설정하면 더 빨리 인지할 수도?

      → 기존에는 warn으로 되어있음

• PR을 올릴 때 주석 달기

  • 리뷰어에게 도움이 됨

  • PR을 저자가 먼저 읽어 보고 리뷰어들을 위한 설명을 코멘트로 남겨서 리뷰어들의 시간을 절약

    → 자신이 원하는 피드백 등을 코멘트로 남겨두자

• 모두를 포함하라(팀 전체 인원)

  • 많은 사람들이 볼 수록 버그를 더 잘 찾아냄
  • 많은 사람들이 본다는 것을 알면 더 잘 하려는 경향이 있음

• 의미있는 커밋으로 분리

  • 별도의 커밋, PR로 분리
  • 리뷰 불필요를 기술해서 리뷰를 생략할 수 있도록
  • ex) formatting 커밋 등은 리뷰를 할 필요가 없으니 별도의 커밋으로 분리 → 리뷰어의 시간을 아껴줌
  • 커밋을 작게 나누더라도 의미 있는 커밋으로 나누는 게 좋다

효율적인 리뷰 방법

• 리뷰는 즉시 시작

  • 코드 리뷰를 높은 우선순위로

    • 저자는 종료될 때까지 대기함(blocked)

  • 리뷰를 바로 시작하면 선순환됨

    • 코드를 읽고 피드백을 줄 때는 시간을 가져도 되지만 시작은 바로
    • 이상적으로는 수분 내에
    • 리뷰 라운드의 최대 시간은 하루
    • 우선순위 높은 업무로 1일 내 불가하면 다른 리뷰어 지정
    • 월 1회 이상 재지정을 해야한다면 속도를 줄여 건강한 개발 관습을 유지할 수 있도록
  • 아침 30분 점심 식사 후 30분을 리뷰를 위해 미리 확보하는 것처럼 시간을 미리 확보

  • PR에 포함된 변경이 적도록 노력

    • 반나절 정도 작업한 양 정도
    • 모든 팀원들이 PR을 리뷰할 수 있고 최대 4시간 안에 완료될 것

  • 근본적인 문제는 사람들이 리뷰할 시간이 없다고 느끼는 것

    • 팀을 위해 수행하는 일은 시간 낭비처럼 느낌
    • 리뷰를 하는 것의 문제라기보다 조직적인 문제

• 고수준으로 시작, 저수준으로 내려감

  • 리뷰 라운드에서 많은 의견을 남길 수록 저자가 당황할 위험 커짐

  • 초기 라운드에서는 고수준 피드백으로 제한

    • 버그, 장애, 성능, 보안 등
    • Extract Method, Composed Method, Invert-if(복잡도) 등

  • 고수준 피드백 이후 저수준 이슈를 처리

    • 설계 개선
    • 변수명 변경, 주석을 명확히 하는 것 등

• 예제 코드 제공

  • 저자를 기분 좋게 하기 위한 방법
  • 예제 코드를 통해 리뷰 중에 선물 주기

  • 너무 긴 예제는 관대한 것 보다 억압적으로 보임

    • 너가 코드를 못 짜니 내가 다 짜서 주겠다.
  • 라운드당 2~3개의 코드 예제로 제한

• 리뷰의 범위를 존중

  • 자주 보이는 Anti-Pattern

    • PR 근처의 코드를 보고 저자에게 수정을 요청
  • Rule of thumb

  • 리뷰를 통해 코드가 크게 변할 수 있음

    • 레거시 코드를 통한 오류, 기능 삭제에 따른 함수명 변경 등

• 태그를 활용

  • Nit - 하찮은 일로 트집 잡기
  • 고치면 좋지만 아니어도 그만 을 의미
  • 리뷰어는 항상 더 개선할 수 있는 의견을 자유롭게 남길 수 있어야 함
  • 중요치 않다면 ‘Nit’ 태그로 저자가 무시할 수 있도록 할 수 있음
  • 교육적인 목적, 지속적으로 기술을 연마하는 것을 돕는 목적
  • 예) Nit : null 대신 Optional을 쓰면 어떨까요?

• 한, 두 등급만 올리는 것을 목표로

  • D 등급의 PR을 받으면 저자가 C나 B 등급을 받도록 도와라

    • Letter Grade : 학점으로 등급을 매기는?
  • 완전하지는 않아도 충분히 좋은 코드가 되도록

  • F

    • 기능적으로 틀렸거나
    • 너무 복잡해서 정합성에 확신이 없는 상태

  • 승인을 보류하는 유일한 이유

    • 수 차례의 리뷰 라운드 후에도 코드가 F 상태인 경우

피드백 방법

• 절대 ‘너’ 라고 하지 마라

  • 리뷰의 핵심

    • 코드를 나아지게 하는 것이 중요하지 누가 잘못된 아이디어를 냈는지는 중요하지 않음
    • 저자의 방어 유발을 최소화하는 방법으로 피드백

    • 너만 빼라

      • 저자에 대한 판단 → 단순한 정정

• 건설적인 피드백을 하라

  • 동료들간의 코드 리뷰는 경쟁을 유발하는 것이 아닌 팀의 생산성을 높이는 것
  • 자신의 코드에 대한 비판이 아니라 학습의 과정으로 인지
  • 건설적인 피드백은 개발자들이 그들의 실수에서 배우고 역량을 증대하도록 동기부여함
  • 건설적인 피드백을 하지 못하겠으면 그냥 하지 마라

• 진정한 칭찬을 해라(결함만 찾지 마라)

  • 대부분의 리뷰어가 잘못된 부분에만 집중

    • 리뷰는 긍정적 행위 강화를 위한 값진 기회이기도 함
  • 저자가 주니어 혹은 신규 입사자라면 리뷰에 매우 민감하고 방어적일 수 있음

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

  • 일상에서 동료에게 명령하지 않음
  • 하지만 리뷰에서 강압적인 명령이 종종 발견됨

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

  • 저자에게 의견을 줄 때는 제안하는 변경과 변경의 이유를 모두 설명하라
  • 항상 원칙에 기반하여 정확히 뭐가 잘못 되었는지 언급할 수 있는 것은 아님
  • 무엇을 살 수 있을지 객관적으로 설명하라

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

  • 저자의 실수가 동일한 패턴임을 식별했다면 모든 경우를 언급하지 말자
  • 동일 패턴에 대해서 2~3개 정도의 예를 언급
  • 그 이상은 저자에게 개별 사례가 아니라 패턴에 대해서 수정을 요구

교착상태 시

• 교착상태를 적극적으로 철리해라

  • 교착상태로 향하는 표식

    • 토론의 톤이 점차 팽팽해지고 공격적으로 됨
    • 라운드당 커멘트가 줄어들지 않는 경향
    • 너무 많은 커멘트에 저항이 보임

  • 코드 리뷰의 최악의 결과는 교착상태

    • 커멘트를 반영하지 않으니 승인 거부
    • 저자는 커멘트 수용 거부
  • 만나서 얘기해라

  • 인정하거나 Escalate하라

    • Agree to Disagree
    • 교착상태가 길어지면 관계가 나빠짐
    • 그냥 승인하는 비용
    • 그렇다고 저수준 코드를 무심코 승인하면 SW품질이 낮아질 수 있음

  • 인정이 불가한 경우

    • 저자에게 논의를 관리자나 리더에게 escalate
    • 다른 리뷰어에게 할당

  • 교착상태로부터 회복

    • 관리자와 상황을 논의
    • 휴식을 가져라
    • 갈등 해결책을 학습
  • 설계 리뷰를 고려하라

• 코드 리뷰를 재밌게 하는 방법

  • PR을 작성한 사람과 짝 프로그래밍을 하며 어떻게 고치는 게 좋은지 보여주고 Revert → 씹악질
  • PR을 작성한 사람이 스스로 개선할 수 있도록 기회를 줌
  • 그래야 스스로 하는 법을 배움

  • 결정은 저자가

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

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

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

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

    • 저자가 그날 무슨 일이 있을 수도..?

Appendix

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

• 저자의 노력이 중요함!

  • 리뷰어 n명의 시간을 절약
  • 효과적인 리뷰 가능

• 리더의 관심과 의지

  • 가끔 그러나 매우 자세히
• 코드 비난에 대한 어려움을 없애야 함
• 멋져 보여야 하고 싶어짐

• 모범이 되자

  • 모범이 되어야 남에게 영감을 줄 수 있다

코드 리뷰 효과

• 예상하지 못한 버그를 타 프로젝트에서 발견하기도
• 시간이 지나니 선플이 달리기 시작
• 많은 사람이 내가 작성한 코드를 본다는 생각에 PR 전에 한번 더 코드를 다듬게 됨
• 좋은 설계, 아키텍처, 클린코드, TDD 등에 대한 동기부여를 받을 수 있음

리팩토링

• TDD와 연결된 리팩토링

  • 돌아가는 코드를 가지고 설계를 하는 행위
  • 그린필드 프로젝트(아무것도 없는 프로젝트)에서 아무것도 없는 상태에서 분석/설계부터 시작하는 비율

• 매일 리팩토링

  • Continuous Renaming
  • Composed Method(Extract Method)
  • 응집도 높이기
  • Value Object 활용

Legacy 코드 다루기

• 의존성 관리
• 테스트 추가
• 새로운 코드 빠르게 추가
• 레거시 분석

FAQ

• 코드 리뷰 자체가 중요할까?

→ 그것은 아님, 공유와 코드에 대한 논의를 할 수 있는 문화가 중요

• 개발 생산성 vs 개발 품질의 tradeoff

→ 버그 수정 비용은 개발 라이프 사이클 후반으로 갈 수록 기하급수적으로 커짐

품질이 높으면 라이프 사이클의 후반으로 갈 수록 시간이 절약됨

따라서 TDD, Test, Refactoring, Code Review 등은 생산성을 증대시킴

높은 품질은 향후 추가 비용을 감소시킴

• 주기는 짧게 자주하는 것이 좋음

→ PR 사이즈를 작게 하자

Q&A

• 회사 규모에 따른 코드 리뷰 방법

  • 보통 팀 단위, 팀 내에서 같은 일을 하는 사람들과 리뷰 진행
  • 여유가 되면 다른 팀의 코드도 리뷰..?

• 초기 → 성숙으로 갈 때의 단계

  • 작은 단계부터 시작

• Non-tech 기업에서 비전공자, 비개발자들과 어떻게 리뷰를 진행할까?

  • 관심으로 시작
  • 이해를 할 수록 도와주자

Etc

PR vs Pair Programming

latency와 throughput 사이의 tradeoff가 존재

PR은 latency가 있다 → 리뷰 시간

pair programming은 옆에서 바로 확인하므로 리뷰 시간이 없음 → latency가 없음

PR은 여러 명이 확인하므로 throughput이 좋음

pair programming은 PR에 비해 throughput이 떨어짐(인원이 비교적 적기 때문에)

내성적, 사색, 비동기 → PR

외향적, 친밀한 개인적 상호 작용 → pair programming

절대 답은 없다.