The Standard of Code Review

본 문서는 google code review guide를 보고 요약한 글 입니다.

• 코드 작성자가 자신의 작업을 진행할 수 있어야 합니다.
  만약 코드 작성자가 개선된 code를 commit하지 않는다면, 코드베이스는 절대 개선되지 않습니다.
  또한 리뷰어가 리뷰한 사항이 현재 코드에 적용하기 매우 어려운 경우, 코드 작성자들은 향후 개선을 위한 노력을 하지 않게 됩니다.

• 각 CL의 quaility을 검토하는 것은 리뷰어의 의무입니다.
  각 CL이 시간이 지남에 따라 코드베이스 전체의 품질에 영향을 주지 않을지 검토해야합니다.
  특히 팀이 시간제약을 받은 상태에서 목표달성을 위해 빠르게 제품을 만들어야 할 때, 각 CL의 품질과 목표 사이에서 trade off 해야 하기 때문에 더 까다로울 수 있습니다.

• 또한 리뷰어는 리뷰중인 코드에 대한 소유권과 책임이 있습니다.
  코드베이스가 일관성 있고 유지보수가 가능하도록 전체적인 품질을 관리합니다. 또한 What to look for in a code review에 언급된 모든 것들이 잘 적용되어 있는지 검토해야 합니다.

• 일반적으로 리뷰어는 CL의 품질 완벽하지 않다고 하더라도, 작업중인 코드베이스의 상태를 확실히 개선하는 코드인 경우 CL을 승인하는 것에 대해 긍정적으로 생각해야 합니다 (물론, CL이 필요하지 않은 기능을 추가하는경우 그 CL이 완벽하다고 하더라도 리뷰어는 승인을 거부할 수 있습니다).

• 여기서의 핵심은 완벽한 코드는 없다는 것입니다.
  오로지 더 나은 코드만 있을 뿐입니다. 그렇기 때문에 다음과 같은 원칙을 지켜야 합니다.

  1. 리뷰어는 작성자가 CL의 모든 작은 부분을 완벽하게 구현하도록 요구해서는 안됩니다.
  2. 리뷰어는 자신의 리뷰와 앞으로 진행해야할 process의 균형을 유지해야합니다.
  3. 완벽에 대한 추구 보다는 지속적 개선을 추구해야 합니다.
  4. CL은 절대 완벽할 수 없기때문에 며칠 혹은 몇주간 리뷰상태로 지연시켜는 안됩니다.
  5. 리뷰어는 항상 더 나아질 수 있는 가능성에 대한 의견을 자유롭게 남겨야 합니다. 다만 그다지 중요하지 않은 의견의 경우 “Nit:“와 같은 접두어를 붙여줍니다.

1. 멘토링

코드 리뷰는 개발자에게 언어, 프레임워크 또는 일반적인 프로그래밍 원칙에 대해 알려주는(teaching) 중요한 기능을 할 수 있습니다. 그렇기때문에, 코드 작성자가 새로운 것을 배우는데 도움이 되는 의견을 남기는 것이 좋습니다. 지식 공유는 시간이 지남에 따라 코드베이스의 상태를 개선하는데 도움이 됩니다.

만약 어떠한 리뷰가 크게 중요하지 않지만, 기술적으로 배울 점이 있는 경우에는 “Nit:“로 접두어를 붙여, 코드 작성자가 CL에서 이를 해결하는 것이 필수가 아님을 알려줘야 합니다.

2. 원칙

1. 기술적 사실과 데이터는 의견이나 개인 취향보다 우선합니다.

2. 스타일 문제에 대해서는 스타일 가이드가 절대적인 위치를 차지합니다.

   • 스타일 가이드에 없는 스타일은 개인적 취향의 문제입니다. 코드 작성자의 스타일이 우선시 됩니다.
   • 스타일 가이드 항목에 있는 것을 어겼을 경우 스타일 가이드가 우선시 됩니다.

3. 소프트웨어 디자인은 순수한 스타일 문제가 아닌 개인적인 취향에 지나지 않습니다.

   • 코드 작성자가 자신의 접근 방식이 유효하다는 것을 여러 근거(데이터, 견고한 엔지니어링 원칙 등)를 통해 입증 할 수 있다면, 리뷰어는 코드 작성자의 의견을 수용해야 합니다.
   • 만약 그렇지 못하다면, 소프트웨어 디자인의 표준 원칙에 따라 수용 여부를 결정해야 합니다.

4. 위의 규칙이 적용되지 않는다면, 코드베이스 전체의 품질을 떨어뜨지 않는 선에서, 리뷰어는 현재 코드베이스의 내용과 CL의 일관성을 유지할 수 있도록 코드 작성자에게 요청 할 수 있습니다.

3. 갈등 해결

1. 첫 번째 단계는 리뷰어와 코드 작성자가 이 문서, 혹은 다른 규칙에 대한 문서와 규약을 기반으로 합의를 시도하는 것입니다.

2. 두 번째 단계는 리뷰어와 코드 작성자가 회의 또는 VC(Voice Chat)를 갖는 것이 좋습니다. 토론 결과를 향후 다른 개발자들을 위해 CL의 message에 남기도록 합니다.

3. 그래도 해결이 되지 않는다면 이 이슈를 이관하도록 합니다. 광범위한 팀 토론, TL 계량, 코드 관리자에게 결정 요청 등을 하면 됩니다.

Don’t let a CL sit around because the author and the reviewer can’t come to an agreement.