구글 엔지니어는 이렇게 일한다

09. 코드 리뷰

주요 내용

• 코드 리뷰의 이점이 많음 - 정확성, 이해 용이성, 일관성 보장 등
• 코드 리뷰는 조직의 지식 공유에 기여함
• 코드 리뷰 프로세스 확장을 위해서는 자동화가 중요
• 코드 리뷰 자체가 변경 이력임
• 모범사례가 엄격히 지켜져야 개발자의 만족도와 생산성이 유지됨

이번 장에서는 코드 리뷰 프로세스에 대해 이야기합니다. 코드 리뷰란 작성자 이외의 사람이 코드를 검토하는 프로세스로, 주로 코드를 코드베이스에 반영하기 전에 수행합니다. '버그가 코드베이스로 침투하기 전에 잡아낸다'라는 이점 외에도 다양한 코드 리뷰의 장점을 확인하고, 필요성에 대해 이해합시다.

1. 코드 리뷰 흐름

코드 리뷰는 SW 개발 단계 어느 곳에서나 가능합니다. 구글은 변경을 코드베이스에 커밋하기 이전에 수행합니다. 이를 precommit review라고 합니다. 코드 리뷰의 최종 목표는 다른 엔지니어로부터 해당 변경을 적용해도 된다는 합의를 이끌어내는 것입니다. 구글에서는 이를 LGTM 이라는 태그를 달아 표현합니다.

LGTM (Looks Good to me)
to me: 각자가 자신이 맡은 영역에만 한정해서 리뷰한다는 의미
looks good: 코드의 완벽한 품질보다는 개선을 중시하는 구글의 문화에서 비롯된 것
즉, 코드가 완벽하지 않더라도 전체적인 품질을 높이는 상태에 도달했다면 승인하는 방향 지향

구글의 코드 리뷰 절차

• 작성자가 변경사항을 작성한 후 스냅샷을 생성합니다. 스냅샷이란 코드 리뷰 도구에 업로드할 코드 패치와 관련된 설명입니다.

• 한 명 이상에게 리뷰를 요청합니다.

• 리뷰어들은 diff에 댓글로 의견을 남깁니다. (수정 사항, 유용한 정보 등)

• 작성자는 피드백을 기초로 변경을 수정하고 새로운 스냅샷을 업로드해 리뷰어들에게 회신합니다.

• 리뷰어들이 변경 내용에 모두 만족하면 LGTM 태그를 달아줍니다. 보통은 모든 리뷰어가 LGTM을 달아주는 것이 관례지만 원칙적으로는 한 개만 얻어도 됩니다.

• 모든 댓글에 LGTM이 달려 변경이 승인 되면 작성자는 커밋할 수 있습니다.

2. 코드 리뷰 @ 구글

구글에서는 어떤 변경이든 '승인'을 얻으려면 세 가지 측면의 리뷰를 통과해야 합니다.

• 정확성(correctness)과 이해 용이성(comprehension) 평가
  필수는 아니며, 팀원이 해주는 경우가 많음.

• 코드 영역을 관리하는 코드 소유자로부터의 승인
  작성자가 코드 소유자라면 자동으로 승인됨. 구글의 코드베이스는 트리 구조이며, 디렉토리별 소유자들이 계층적으로 할당되어 있음. 소유자는 테크 리드나 해당 영역의 기술 전문가가 맡음. 코드베이스에 변경을 반영하려면 해당 디렉토리 소유자의 승인이 반드시 필요.

• 가독성 승인
  해당 언어 가독성 인증자가 승인하며, 작성자가 가독성 인증자라면 자동으로 승인됨.

과해보이지만 대부분의 리뷰는 세 역할을 모두 수행할 수 있는 한 명이 처리하므로 빠르게 진행됩니다. 세 가지 승인 조건은 어떤 형태로 조합되든 상관 없습니다.
만약 승인을 두 개 이상 얻어야 하는 코드 리뷰는 대부분 동료로 부터 LGTM을 받은 후, 해당 코드 소유자와 가독성 인증자에게 승인을 요청합니다. 이를 통해 두 역할의 사람들이 각기 다른 측면에 집중해 리뷰하도록 해줘 시간을 절약해 줍니다. 또한 승인 측면을 셋으로 나눔으로써 코드 리뷰 프로세스가 더 유연해져 확장성이 생깁니다.

소유권
구글에서는 리포지터리의 지식과 책임을 소유권(Ownership)이라 하고, 소유권을 행사하는 사람을 소유자(Owner)라고 합니다. 회사가 추구하는 가치가 지켜지도록 관리한다는 의미의 소유입니다. 리포지터리에는 OWNERS 파일을 담고 있는 디렉토리들이 있고, 해당 파일엔 디렉토리별 소유자 이름이 나열되어 있습니다.
코드 소유권으로 코드 커밋을 승인해줄 사람을 알릴 수 있습니다. 소유자는 대규모 변경을 책임지는 전역 승인자 역할을 맡을 수도 있습니다. 또한 OWNERS 파일을 통해 사람이나 도구가 책임자를 찾기 쉬워집니다.

3. 코드 리뷰의 이점

구글은 아무리 작더라도 코드베이스를 수정하는 거의 모든 변경에 코드 리뷰를 요구합니다. 이런 강제적인 규제는 비용을 유발하고 엔지니어링 속도에도 영향을 줍니다. 하지만 다음과 같은 이점 때문에 장기적으로 봤을 때는 더 효율적입니다.

• 코드 정확성
  변경된 코드의 정확성을 확인할 수 있고 소프트웨어 버그를 예방하는 효과가 있습니다. 이때 코드 리뷰 프로세스 자체는 단순화하여 가볍게 유지해야 합니다. 정확성 평가가 객관적이기 위해 변경자의 방식을 존중해야 합니다. 리뷰어는 자신이 선호한다는 이유로 다른 안을 주장해서는 안됩니다. 덜 복잡하거나 더 효율적인 대안이 있을 경우에만 제시할 수 있습니다. 코드 리뷰는 만능 치트기도 아니고, 정확성 검사의 유일한 수단도 아니므로 완벽할 필요는 없습니다.

• 코드 이해 용이성
  타인으로 부터 작성자의 관점에 치우치지 않은 피드백을 받을 수 있는 기회입니다.

• 코드 일관성
  규모가 커질수록 코드는 읽히는 시간이 더 길며, 다른 사람이 유지보수 하게 될 것입니다. 따라서 리뷰를 통해 코드가 건실함(health)을 보장해야 합니다. LGTM을 받은 상태와 가독성 승인을 분리한 이유는 유지보수성 때문입니다. 코드가 일관되고 단순할 수록 사람들이 쉽게 이해하고 리팩토링하기도 편리해집니다.

• 심리적/문화적 이점
  코드는 '자신의 것'이 아니라 협업을 통해 만들어지는 '조직의 공동 소유물'임을 인식시켜주는 효과가 있습니다. 다른 사람의 피드백을 받고, 더 큰 이익을 위해 적절히 타협할 수 있도록 합니다. 비판적 피드백도 코드 리뷰를 통해 기분 좋게 받아들일 수 있습니다. 또한 타인으로부터 검증을 받음으로써 좋은 코드라고 인정해주는 효과가 있습니다. 마지막으로 코드 리뷰를 위해 자신의 코드를 다시 확인하게 하여 더 완벽한 코드를 작성하도록 돕습니다.

• 지식공유
  코드 리뷰는 지식 공유의 기회입니다. 리뷰어는 FYI(For Your Information)라는 주석으로 프로세스 제안, 신기술 소개, 조언 등을 통해 도메인 지식을 전파합니다. 피드백을 주고 받으며 정보가 교환되고 지식 공유를 촉진시킵니다.

4. 코드 리뷰 모범 사례

• 공손하고 전문가 답게
  작성자가 다른 대안 중 특별히 더 우수한게 없음을 설명할 수 있다면 리뷰어들은 작성자의 취향을 받아들여야 합니다. 작성자가 선택한 방식에서 결함을 찾게 되면 (작성자와 리뷰어 모두)배움의 기회로 생각하면 됩니다.
  리뷰어는 신속하게 피드백해야 합니다. 작성자는 피드백에 감정적으로 대응해서는 안되며, 리뷰어가 단 댓글은 모두 TODO로 취급해야 합니다. 댓글을 의문 없이 수용할 필요는 없지만, 동의하지 않는 다면 리뷰어에게 그 이유를 설명해야 합니다.

• 작게 변경하기
  변경을 가능한 작게 만들어야 합니다. 작은 변경이란 약 200줄 이하의 코드를 뜻합니다. 코드 리뷰를 작게 해야 확장성을 유지할 수 있습니다. 또한 롤백에도 용이하며, 빠른 리뷰를 할 수 있습니다.

• 변경 설명 잘 쓰기
  변경 설명의 첫 줄은 어떤 종류의 변경인지를 잘 요약해야 합니다. 구체적으로 무엇을 왜 변경하는지도 작성해야 합니다. 연관된 수정 여러 개를 포함했다면 그 모두를 간략하게 열거합니다. 코드 주석을 통해서도 자세히 설명할 수 있습니다.

• 리뷰어는 최소한으로
  사공이 많으면 배가 산으로 갑니다. 첫 번째 LGTM이 가장 중요합니다.

• 가능한 자동화하기
  프로세스 중 자동화할 수 있는 요소가 있다면 자동화를 해야합니다. 이를 통해 더 효율적인 코드리뷰가 가능해집니다.

5. 코드 리뷰 유형

코드 리뷰 유형에 따라 리뷰 프로세스의 어느 측면에 더 집중해야 하는지가 달라집니다. 다음과 같은 네 가지 분류가 있으며 때로는 겹치기도 합니다.

• 그린필드 코드 리뷰와 새로운 기능 개발
  Greenfield review는 완전히 새로운 코드를 대상으로 하는 가장 드문 유형의 코드 리뷰입니다. 코드는 부채(Debt)이므로 오래 존속될 수 있는지, 유지보수 하기 쉬운지 등을 확인해야 합니다. 코드 리뷰때는 이미 결정된 설계에 관해서는 얘기해서는 안되고, 단지 API가 합의된 설계에 부합하고 테스트가 완벽히 이뤄졌는지에 대해 확인해야 합니다. 주석도 충분해야 하고, 필요하다면 보충 문서도 제공해야 합니다.

• 동작 변경, 개선, 최적화
  꼭 필요한 변경인지, 코드베이스를 개선하는지를 살펴햐 합니다. 최적화를 할 경우에는 벤치마크 테스트를 준비하는 것이 좋습니다.

• 버그 수정과 롤백
  버그를 수정하면서 기능 변경 등 다른 문제까지 묶어서 처리해서는 안됩니다. 한편 테스트 코드 보강도 이뤄져야 합니다. 기존 테스트 케이스가 충분히 커버하지 못했기 때문에 버그가 새로 발견된 것입니다.

• 리팩터링과 대규모 변경
  기계에 의해 많은 변경이 일어날 수 있습니다. 이런 변경도 리뷰가 필요합니다. 하지만 이 때는 댓글 작성을 지양합니다. 또한 기반 도구나 변경을 생성한 LSC(대규모 변경) 자체에는 신경쓰지 말고 리뷰어 자신의 코드와 관련된 문제만 신경써야 합니다.