백명석님의 지속가능한 SW 개발을 위한 코드리뷰

소프트 웨어 장인정신이란?

• 지식과 경험의 공유로 전문성을 갖춘 개발자를 육성
• 후배들에게 지식과 경험을 공유

코드리뷰

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

목적?

• 주목적은 품질문제 검수
• 학습 및 지식 전달
  • 코드, 해결책 등 관련 지식 공유에 기여
    • 대게 리뷰어들도 하드 스킬과 소프트 스킬도 얻게 된다.
  • 다른 사람이 잘 하는 모습을 보고 동기부여가 잘 된다.
• 상호 책임감 증대
  • 오너십 및 결속 증대
  • 팀에서 일어나는 일 공유
• 설계 개선 제안
• 개발 문화 개선

코드리뷰가 어려운 이유

• 좋은 PR을 만들기 위해서 PR의 저자가 고생해서 설명을 적어주어야 한다.
  • 만드는 사람이 고생해야 쓰는 사람이 편하다와 같은 논리
• 코드에 대한 비판을 자신에 대한 비판으로 이해
  • 코드의 대한 토의는 공유를 통한 상호 학습의 수단이다.
    • 개인적 공격으로 받아들이면 물거품
• 생각을 그로 전달하는 것에 대한 어려움
• 피드백을 조심스럽게 표현하는 것이 더 중요하다.
  • “파일을 닫는걸 잊어버렸네요?”
    • “바보도 아니고 이걸 어떻게 잊어버렸지?”

효율적인 PR 방법

• 코드를 읽는 것은 인지적 부담이 되는 고수준의 집중이 요구된다.
  • 리뷰에 불필요한 부분은 가능한 덜어내자
    • 코드 포맷팅, 스타일 등
• PR을 올릴 때 주석달기
  • 리뷰어들을 위한 설명을 코멘트로 남겨서 리뷰어의 시간을 절약할 수 있게하면 좋다
• 모두를 포함하라
  • 많은 사람들이 볼 수록 버그를 더 잘 찾아낼수 있다.
    • 많은 사람이 보면 더 잘하려는 경향이 있다.
• 의미있는 커밋으로 나누어라
  • 혼자 개발하는 경우 2주 뒤의 나에게는 나도 타인이 될 수 있다.

효율적인 리뷰 방법

• 리뷰는 즉시 시작
  • 코드리뷰는 높은 우선순위를 가져야한다.
    • 저자는 리뷰가 종료될 때 까지 대기해야하는 경우가 있을 수 있다.
  • 리뷰를 바로 시작하면 선순환이 된다.
    • 언제 어떻게 올려야 더 빠르게 리뷰가 되는지 파악이 된다.
  • 리뷰 라운드는 최대한 하루로 잡도록 하라
    • 하루내에 리뷰를 못할것 같으면 다른 리뷰어를 지정하도록 하라
      • 재지정 빈도수가 늘어난다면 속도를 줄여서 건강한 개발관습을 유지할 수 있어야 한다.
  • 매일 일정 시간(오전, 오후)을 리뷰를 위해 확보 하도록 하라
  • PR에 변경이 적도록 노력
    • 반나절정도 작업한 양
      • 모든 팀원이 하루에 두번(오전, 오후) 작은 양의 PR을 리뷰할수 있도록
      • 최대 4시간 안에 리뷰가 완료될 수 있도록
  • 그런데 사람들이 리뷰할 시간이 없다고 느낀다.
    • 코드리뷰 행위가 개인기여로 평가받는 다면 팀을 돕기 위해 수행하는 모든 일이 시간낭비처럼 보일수 있다.
    • 리뷰를 행하는 것이 문제가 아니라 가치를 인정해주지 않고 있는 조직적인 문제이다.
  • PR vs 페어코딩
    • 트레이드 오프
• 고수준으로 시작, 저수준으로 내려가라
  • 너무 많은 리뷰가 남겨지면 저자가 당황할 수 있다.
    • 20~50개가 온다면 위험한다
  • 초기 라운드에는 고수준 피드백으로 제한하라
    • 버그, 장애, 성능, 보안 등
      • 메서드 추출, 메서드 조합, 복잡도 개선
  • 고수준의 피드백이 처리된 후 저수준 이슈를 처리
    • 변수명 변경, 주석 달기 등
• 예제 코드 제공에 관대하라
  • 저자를 기분 좋게 하기 위한 방법
    • 리뷰 중에 선물 주기(코드 예제)
  • 다만 너무 긴 예제는 억압적으로 보일수 있다.
  • 라운드 당 2~3개의 코드 예제로 제한
    • 너무 많은 예제를 제공하면 저자가 코드를 작성 할 수 없다고 생각하게 할 수 있다.
• 리뷰의 범위를 존중하라
  • PR 근처의 코드를 저자에게 수정을 요청하는 것은 안티패턴이다.
  • PR에 포함되지 않은 라인은 리뷰 범위가 아니다.
  • 단 PR이 둘러싼 코드에 영향을 미친다면 리뷰의 대상이 될 수 있다.
• 태그를 활용
  • Nit
    • ‘고치면 좋지만 아니어도 그만’
    • 리뷰어는 항상 더 개선할 수 있는 의견을 자유롭게 남길 수 있어야 한다.
    • 중요치 않으면 Nit 태그로 저자가 무시할 수 있도록 할 수 있다.
    • 교육적 목적, 지속적으로 기술을 연마하는 것을 돕는 목적
      • null 대신 Optional을 쓰는게 어떨까요?
      • OCP 준수를 위해 strategy 도입은 어떨까요?
• 한 두 등급만 코드 레벨을 올리는 것을 목표로
  • D등급 PR을 받으면 저자가 C, B 등급을 받도록 도와라
    • Letter Grade
      • 모든걸 알려준다고 모든걸 습득하고 따라가지 못한다.
  • F등급 PR이면?
    • 기능적으로 틀린경우
    • 너무 복잡해서 정합성에 확신이 없다면?
    • 이때는 승인을 보류하라

피드백 방법

• 너, 왜, 맨날 같은 단어 사용금지
  • 리뷰의 핵심
    • 누가 잘못 했는지가 아니다
    • 무엇이 코드를 나아지게 하는가
  • 저자의 방어 유발을 최소하 하는 방법으로 피드백하라
    • 비판의 대상은 코드이다.
  • ~하는게 어떨까요?
    • 오픈 커뮤니케이션
• 건설적인 피드백을 하라
  • 동료간 코드리뷰
    • 경쟁이 아니다
    • 생산성을 높익는것
  • 비판이 아니라 학습의 과정으로 인지하여 전체적인 프로젝트의 성공에 기여
  • 건설적인 피드백으로 실수에서 배우고 역량을 증대하도록 동기부여
• 진정한 칭찬을 하라
  • 리뷰어는 잘못된 부분에만 집중하기 때문이다.
    • 저자가 자괴감에 빠지지 않도록
  • PR에서 좋은 변경이 있을 때 마다 칭찬하도록 하자
    • “오 이런 API가 있나요? 유용하네요”
    • “함수를 분리한 것은 좋은 생각이네요. 훨씬 단순해졌어요”
  • 저자가 주니어, 신규입사자라면 리뷰에 매우 민감하고 방어적일 수 있다.
  • 진심어린 칭찬은 리뷰어가 잔인한 감시자가 아니라 도와주려는 팀 동료라는 것을 보여서 긴장감을 낮추게 한다.
• 피드백은 명령이 아니라 요청으로 표현하라
  • 일상에서 동료에게 명령하지 않는다. 코드리뷰도 동일하다
• 의견이 아니라 원칙에 기반하여 피드백하라
  • 제안하는 변경과 변경의 이류를 모두 설명하라
  • SW는 과학인 동시에 예술
    • 단지 보기 싫거나 직관적이지 않은 코드를 보게된 경우
      • 무엇을 할 수 있을지 객관적으로 설명하라
        • 이 코드는 혼란스럽네요. → 나는 이 코드를 이해하기 어렵네요.
• 반복적인 패턴에 대해서 피드백을 제한하라
  • 저자의 실수가 동일한 패턴임을 식별했다면 모든 경우를 언급하지는 말라
    • 동일 패턴에 대해 2~3개 정도 언급하라
    • 개별사례가 아닌 패턴을 언급하라

교착상태 시

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

추가적인 사례

• PR을 작성한 사람과 짝 프로그래밍을 한다.
  • Revert를 한다.
    • PR을 작성한 사람이 스스로 개선할 수 있도록 기회를 주자
      • 시간이 오래 걸릴지언정 스스로 하는법을 배우게 된다
• 결정은 저자가
  • 완벽한 설계가 아닌 당신이 할 수 있는 최고의 설계를 추구하라
    • Letter Grade
  • 팀 정신을 유지하기 위해 불완전한 해결책을 받아들여라
  • 모든 설계 결함이 항상 실제로 문제가 되지 않는다.
• 코드리뷰의 목적은 비난이 아닌 배움
• 리뷰하려는 코드가 그냥 나쁠 때가 있음
  • 저자에게 무슨일이 있었을지도
  • 좀 더 안내하는 가르침으로 전환

Appendix

• 저자의 노력 (생산자의 노력)
  • 리뷰어 n명의 시간을 절약 (소비자를 위해)
  • 효과적인 리뷰 가능
• 리더의 관심과 의지
  • 가끔, 그러나 매우 자세히
• 코드 비난에 대한 두려움
• 따라하고 싶도록 코드 리뷰를 하자
• 코드 리뷰같은 것을 어떻게 팀에 도입시킬 수 있는가?
  • 못할수도 있으니 좌절한 준비가 필요
  • 타인에게 내가받은 영감을 줄 수 없다.
    • 영감은 부산물이다.
      • 내가 어떻게 하면 모범이 될 수 있을까?
        • 특정 행동, 특정 기술을 채택하도록 하려면 모범이 되어야한다.
        • 해당 기술을 마스터 해야한다.
  • 그러니 모범이 되어야 한다.
    • 지각하지 말라. 업무에 기여하라. 긍정적으로 임해라. 사람들을 잘 대하라.
    • 기술적 훈련에 대해 언급하려면 고도로 숙달되어야 한다.
    • 키보드로 코딩 할 때 사람들이 경악하도록 해라
  • 이러한 엄청난 모습을 보여주면 몇몇 사람에게 영감을 줄 수 있을지 모른다.

FAQ

• 코드리뷰 자체가 중요한가?
  • 공유와 코드에 대한 논의를 할 수 있는 문화가 중요
• 개발 생산성 vs 개발 품질의 트레이드 오프
  • 품질이 높으면 라이프싸이클의 후반으로 갈 수록 시간이 절약된다.

Q&A

• 코드리뷰의 성과 측정 방법?
  • 엑셀러레이터 책을 통해 성과 보고가 하나의 방법
• PR을 짧게 자주 하라고 하는데 기능단위로 작업 후 PR을 하면 코드량이 어쩔 수 없이 많아진다. 어떻게 하는 것이 좋은가?
  • 커밋단위로 볼 수 있도록 커밋을 의미있게 작성하도록 노력하라.