지속 가능한 SW 개발을 위한 코드리뷰

개요

얼마 전 유튜브를 보던 중 ‘우아한 Tech’ 체널에서 강의한 코드리뷰 강의가 알고리즘에 의해 추천이 되었습니다.

• https://youtu.be/ssDMIcPBqUE

해당 영상을 보고 필요한 내용을 정리하여 포스팅하였습니다.

이번 포스팅은 개조식으로 정리하겠습니다.

왜 코드리뷰를 해야하나?

우리가 살고있는 시대

• 비지니스 성공을 위한 개발의 역할
  • 시장 : VUCA
    • Volatliy (변동성) : 변화의 속도가 빠르고 다양하게 전개될 것
    • Uncertainty (불확실성) : 미래 상황에 변수가 많아 예측하기 어려울 것
    • Complexity (복잡성) : 인과관계가 단순하지 않고 다양한 요인이 작용할 것
    • Ambiguity (모호성) : 뚜렷한 현상이 없어 판별하기 어려울 것
    • 불확실하고, 복잡하고, 모호하며, 변화가 많은 세상이 될 것
  • 비지니스 : 더 빨리 혁신해야 함
  • 개발 : DRFR (Deliver SW Rapidly, Frequently and more Reliably)
• 개발 조직의 성능(생산성)이 더 중요해짐

출시별 개발 리소스 증가

• clean architeture 1장 내용
• 제품 출시(배포)가 증가할수록 개발 리소스가 증가함 (엔지니어 수 증가)

동일 기간별 생산성

• 제품이 출시(배포)할수록 생산성은 점점 증가하지 않음 (라인 수로 확인)

출시가 증가함에 따른 라인당 개발 비용

• 제품이 출시(배포)할수록 이에 따른 라인당 개발 비용이 증가함

출시별 생산성

• 기존에 있던거를 수정해나가기에 생산성이 줄어듦
• 점점 기존 코드를 유지하는데 비용이 들어 코드 생산성이 떨어짐

개발 생산성

• 위 그래프는 시간 대비 누적 생산량을 나타냄
• 주황선은 좋은 설계일 때, 파란선은 나쁜 설계일 때임
• design payoff line 이전까지는 나쁜 설계가 생산성이 좋음
• 하지만 이후부터는 좋은 설계일 때 생산성이 올라감

SW 공학 특성

• 공학 활동의 최종 목적
  • 빌드할 수 있는 어떤 종류의 재생산 가능한 문서
• SW 공학의 설계와 빌드
  • 설계 = 완전한 소스코드
  • SW 빌드 = 컴파일
• SW는 설계 비용이 비싸지만, 빌드 비용은 무료나 다름없음
• 좋은 설계 : 클린 코드

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

• 클린 코드의 중요성
  • SW 의 진짜 비용 : 유지보수 (전체의 80% 이상)
  • 한번 작성한 코드는 10번 이상 읽게됨, 작성보다 이해에 10배의노력이 소요
  • 90% 이상의 시간은 어떤 코드를 이해하는데 사용함

SW 장인정신

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

• 학교에서 따로 배우지는 못하고 지식과 경험의 공유만이 전문성을 갖춘 개발자를 육성함

• 코드 리뷰
  • 개발자가 지금부터 당장 행할 수 있는 공유 활동
  • Code SNS 댓글 놀이
  • 배움을 주고 받으며 지속 가능한 SW 개발자가 될 수 있는 실천법

코드리뷰의 목적

• 주 목적 : 품질 문제 검수 (버그, 장애)
• 더 나은 코드 품질 : 아키텍쳐 속성 개선을 위한 코드 개선 (향후 변경 비용 개선)
• 학습 및 지식 전달 : 코드, 해결책 등과 관련된 지식 공유에 기여
  • 학습 및 지식 공유를 통한 역량 증대 및 성장
    • 대개의 경우 리뷰어들도 리뷰 과정에서 지식을 얻게 됨 (하드 스킬, 소프트 스킬)
      • 하드스킬 : 코드 작성방법이나 구현방법과 같은 기술적 스킬
      • 소프트스킬 : 커뮤니케이션, 리더쉽과 같은 스킬
  • 동기 부여 : 다른사람의 코드를 보고 따라하게 됨
• 상호 책임감 증대
  • 집단 코드 오너십 증대
  • 내가 하고 있는 일에 관심을 가져주는 것
  • 팀에서 일어나는 일 공유 (내가 무엇을 하나? 팀웍)
• 설계 개선 제안
• 개발 문화 개선

코드리뷰의 절차

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

왜 코드리뷰가 어려운가

• 저자
  • 본인 생각에 멋지다고 생각하는 PR을 보냄
• 리뷰어
  • 왜 멋지지 않은지에 대한 장황한 이유를 작성

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

• 코드 리뷰란…
  • 지식 / 공학적 결정을 공유하는 기회
  • 잘한 것, 아쉬운 것 등을 공유하여 서로의 지식/경험을 나누며 상호 학습을 통한 역량 증대 수단
  • 코드 토의를 개인적 공격으로 받아들이면 안됨
• 비드백을 조심스럽게 표현하는 것이 중요
  • “You forget to close the file handle. “ -> “I can’t believe you forget to close the file handle. You’re such an idiot.”

기법들

효율적인 PR 방법 (Pull request)

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

• 코드를 읽는 것은 고수준의 집중이 요구되는 작업임
  • 코드 테스트 또는 정적 분석과 같은 작업은 컴퓨터로 처리할 것
• Formatting Tool
  • 공백, 들여쓰기 오류 등
• 11번가 예시로 IntelliJ 옵션으로 사용하지 않는 import가 있으면 컴파일 에러가 나도록 처리함

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

• 스타일에 대한 논쟁은 리뷰에서 시간 낭비
• 사전에 코드 스타일을 미리 공유하고, 이에 맞춰 코딩하는 것이 중요

PR을 올릴 때 주석 달기

1. PR을 저자가 먼저 읽음
2. 리뷰어들을 위한 설명을 커멘트로 남김
3. 리뷰어들의 시간을 절약할 수 있도록 할 것

리뷰어에 모두를 포함하라

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

의미있는 커밋으로 분리

• 위의 그림은 강사님의 커밋 예시임
  • fail test 커밋
  • 테스트 통과하게 수정 후 커밋
  • 테스트에 중복 제거 커밋
  • 불필요한 코드제거
  • 다음기능 개발
• 이와 같이 작은 수정 단위로 커밋을 해야 보는사람이 편함

효율적인 리뷰 방법

리뷰는 즉시 시작

• 코드 리뷰를 높은 우선순위로
  • 저자는 리뷰가 종료될 때 까지 대기함
• 리뷰 라운드의 최대 시간은 하루
  • 우선순위 높은 업무로 1일 내 불가하다면 다른 리뷰어를 지정
• Agile pull requests by Mark Seemann
  • 어려우면 더 자주할 것
  • 매일 아침 30분, 점심 식사 후 30분을 리뷰를 위해 미리 확보
  • PR에 포함된 변경이 적도록 노력
    • 모든 팀원들이 하루에 두번 작은 양의 PR을 리뷰할 수 있고 최대 4시간 안에 리뷰가 완료될 것
  • 근본적인 문제는 리뷰할 시간이 없다고 느끼는 것임
    • 당신의 개인 기여 로만 평가를 받고 있다고 생각하여, 팀을 돕기 위해 수행하는 모든 일은 시간 낭비처럼 보임
    • 이것은 리뷰를 하는 것이 문제라기 보다는 조직적인 문제임
    • 이는 조직적으로 개선하여 리뷰가 가치있는 일이라고 느끼게 해야 함
• Pull Requests VS Pair Programming
  • 트레이드 오프 : Latency or Throughput
  • 내성적, 사색, 비동기를 선호하는 경우 Pull requests 를 선호함
  • 외향적, 친밀한 개인적 상호작용을 선호하는 경우 Pair Programming을 선호함
  • 절대적인 답은 없음

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

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

예제 코드 제공에 관대해라

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

리뷰의 범위를 존중하라

• 수정 범위 외의 코드, 주변 코드도 같이 봐야 함

• 첫번째 예시 리뷰코드
  • eight인데 9로 입력됨
  • 수정한 부분과 관련이 없으므로 넘어갈 것
• 두번째 예시 리뷰 코드
  • validation 로직이 제거되었으며, 이때 함수명도 같이 변경되야 함

태그를 활용

• [Nit]
  • ‘고치면 좋지만 아니어도 그만’ 을 의미
  • 그래도 보통 고침
  • 리뷰어는 항상 더 개선할 수 있는 의견을 자유롭게 남길 수 있어야 함.
  • 중요하지 않다면 Nit 태그를 남겨 저자가 무시할 수 있도록 할 수 있음

피드백 방법

너라고 하지마라

• 비판의 대상은 사람이 아닌 코드
• 리뷰의 핵심은 코드를 나아지게 하기 위함임

건설적인 피드백

• 코드리뷰는 경쟁 유발이 아닌 팀의 생산성을 높이는 것
• 코드리뷰를 코드에 대한 비판이 아닌 학습의 과정으로 인지하도록 해야함
• 건설적인 피드백을 못하겠으면 말을 안하는게 좋음

잘한 부분이 있으면 칭찬도 할것

• 예시
  • “오 이런 API가 있나요? 정말 유용해요”
  • “정말 좋은 해결책이네요. 생각도 못했네요.”
  • “함수를 분리한 것은 좋은 생각이네요. 훨씬 단순해졌어요.”

명령하지 말고 요청으로

• 예시
  • 이 클래스를 별도의 파일로 분리해주세요.
    • 이 클래스를 별도의 파일로 분리할 수 있을까요?
    • 이 클래스는 너무 커지는 것 같은데 괜찮을까요?

원칙에 기반하여 피드백

• “제안하는 변경” 과 “변경의 이유”를 모두 설명하라

• 예시

  • 이 클래스를 2개로 분리해야 해요.
    • 지금 이 클래스는 파일 다운로드와 파싱의 2가지 책임을 가지고 있습니다. 다운로더와 파서 2개의 클래스로 분리하여 SRP를 준수하는 것이 어떨까요?

반복적인 패턴에 대한 피드백 제한

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

교착상태 시

• 교착상태로 향하는지를 나타내는 표식
  • 토론의 톤이 점차 팽팽해지고 공격적으로 됨
  • 라운드당 커멘트가 줄어들지 않는 경향을 보임
  • 너무 많은 커멘트에 저항이 보임
• 교착상태에 빠지는 경우
  • 만나서 얘기하라
    • 화상 또는 만나서 논의 (특히 복잡한 리뷰)
    • 텍스트 기반의 의사소통은 상대가 인간이라는 것을 잊게 함
  • 인정이 불가한 경우
    • 팀장이나 테크 리더와 함께 논의
    • 또는 다른 리뷰어에게 할당

추가적인 사례

• PR 저자와 pair programing을 하여 어떻게 고치는 지 보여줌
• 이후 revert 한 뒤 스스로 개선하도록 함
• 시간은 오래걸리지만 스스로 하는 법을 배울 수 있음

리팩터링

• 결과의 변경 없이 코드의 구조를 재조정함
  • 주로 가독성을 높이고 유지보수를 편하게 함
  • 버그를 없애거나 새로운 기능을 추가하는 행위는 아님
• 리팩터링
  • 코드리뷰시 많은 리팩터링 기술들이 많이 언급됨
  • 응집도 높이기, 함수 나누기 등
• 추천 서적

• TDD와 연결된 리팩터링
  • 돌아가는 코드를 만들고 설계를 하는 행위

레거시 코드 다루기

• 의존성 관리 : Subclass & Override Method
• 테스트 추가 : characterization test
• 새로운 코드 빠르게 추가 : Sprout / Wrap method / Class

• 레거시 분석

• 추천 서적

Clean code & TDD

• 추천 서적