구글의 코드 리뷰 가이드: 변경사항을 작게 나누기

구글의 코드 리뷰 가이드. 코드 작성자는 왜 변경사항을 작게 나누어 리뷰를 받아야 할까?

Posted by : Kimtaeng on Sep 11, 2019

[역주] 이번 섹션에서는 CL(Change List)을 다른 섹션과 동일하게 "변경사항 또는 코드 변경사항" 으로 번역하거나, 원문 그대로 사용합니다.

왜 CL을 작게해야 할까?

작고 간단한 CL은,

리뷰를 빠르게 할 수 있습니다. 하나의 큰 CL을 30분동안 보는 것보다 여러 개로 나뉜 CL을 5분씩 여러번 보는게 낫습니다.
더 철저하게 리뷰받을 수 있습니다. 리뷰어와 코드 작성자는 큰 규모의 변경사항으로 인해 생기는 수많은 의견들로 인해 리뷰의 어려움을 느낄 수 있습니다. 때로는 중요한 요점을 놓칠 수도 있습니다.
버그가 발생할 가능성이 적습니다. 적은 변경사항은 CL이 적용될 때의 영향을 효과적으로 파악할 수 있기 때문에 버그가 생길 요소에 대한 판단을 쉽게 할 수 있습니다.
리뷰가 반려되었을 때의 낭비되는 시간이 적습니다. 만일 당신이 거대한 CL을 작성했는데, 리뷰어가 전반적인 진행 방향이 틀렸다고 리뷰를 반려한다면 당신은 많은 작업을 낭비하게 될 것입니다.
코드를 합치기(merge) 쉽습니다. 대규모의 CL은 시간이 오래 걸리기 때문에 코드를 머지할 때 많은 충돌이 발생하며, 자주 머지를 진행해야 합니다.
더 잘 설계할 수 있습니다. 큰 변경사항의 모든 세부적인 부분을 수정하는 것보다 작은 변경사항의 설계와 코드 품질을 다루는 것이 훨씬 쉽습니다.
리뷰로 인한 업무 진행 차단이 적습니다. 현재 CL을 기다리는 동안 계속 개발할 수 있습니다.
더 간단하게 롤백할 수 있습니다. 거대한 CL은 초기의 CL과 롤백 CL 사이에서 업데이트되는 파일을 수정할 가능성이 높기때문에 롤백을 복잡하게 만듭니다.(중간에 있는 CL들도 롤백을 해야함)

리뷰어는 CL이 너무 크다는 이유 하나만으로도 당신의 리뷰를 완전히 거절할 수 있다는 점을 주의해야 합니다. 대부분 당신의 개발 기여에 고마워하지만 어떻게든 작은 CL로 만들어줄 것을 요청합니다. 이미 거대해진 CL을 분할하는 것은 더 어려울 수 있기때문에 처음부터 CL을 작게 작성하는 것이 더 좋습니다.

작은 CL은 무엇일까?

일반적인 CL의 적절한 크기로는 하나의 독립된 변경사항만 있어야 합니다.

CL은 단지 하나의 변경만 다루는 최소한의 변경사항입니다. 전체 기능 중 일부만을 담고 너무 작아서도 안됩니다.
리뷰어가 CL에 대해서 알아야하는 모든 내용은 CL과 CL 설명문 그리고 코드 베이스 또는 이미 검토한 CL에 있어야 합니다.
CL을 반영한 후에도 시스템이 잘 작동해야 합니다.
새로운 API를 추가할 때에는, 그 API를 사용하는 곳도 동일한 CL에 포함하여 리뷰어가 API 사용법을 더 잘 이해하도록 해야합니다. 또한 사용하지 않는 API는 포함하면 안됩니다.

“너무 크다” 라는 것에 대한 규칙은 없습니다. 일반적으로 100줄 정도가 적합하지만 1000줄은 너무 큽니다. 하지만 이것은 리뷰어의 판단에 달려있습니다. 한 파일에 200줄의 변경은 괜찮지만, 보통 50개의 파일에 분산되어 있는 경우는 좋지 않습니다.

코드를 작성한 당신은 코드에 대해서 잘 알지만, 리뷰어는 전후관계의 맥락을 모르는 것을 명심해야 합니다. 당신은 적절한 크기의 CL로 생각하지만, 리뷰어에게는 부담이 될 수 있습니다. 확실하지 않다면, 자신이 생각하는 것보다 더 작게 작성하면 됩니다. 리뷰어는 너무 작은 CL에 대해서 거의 불평이 없습니다.

언제 큰 CL이 허용되는가?

거대한 변경사항이 나쁘지 않은 몇 가지 상황이 있습니다.

전체 파일을 삭제하는 것은 한 줄의 변경으로 간주할 수 있습니다. 리뷰어가 검토하는데 오래 걸리지 않기 때문입니다.
완전히 신뢰할 수 있는 자동 리팩토링 도구에 의해 생성된 코드도 포함됩니다만 머지 또는 테스트에는 위의 일부 지침이 적용됩니다.

파일로 나누기

CL을 나누는 또 다른 방법은 추가적인 리뷰어가 필요한 사항이지만, 다른 파일로 나누는 것이다.

예를 들어, 프로토콜 버퍼를 수정하는 CL 하나를 리뷰 요청하고, 그 프로토타입을 사용하는 코드 변경을 위해 다른 CL 리뷰를 요청합니다. 프로토타입에 대한 CL을 먼저 요청해야 하지만, 두 개를 동시에 리뷰할 수 있습니다. 두 그룹의 리뷰어에게 각각 다른 CL을 알려서 서로 참조할 수 있도록 하면 됩니다.

또 다른 방법은, 코드 변경에 대한 CL 리뷰를 요청하고, 먼저 요청한 코드를 사용하는 변경사항이 담긴 또 다른 CL 리뷰를 요청합니다. 이는 코드 변경보다 더 빠르게 리뷰가 반영될 수 있기 때문에 필요한 경우 롤백하기도 쉽습니다.

리팩토링 분리하기

일반적으로 리팩토링은 기능 변경이나 버그 수정과 별도의 CL에서 진행하는 것이 가장 좋습니다. 예를 들어, 클래스를 옮기고 이름을 변경하는 것은 해당 클래스에서 버그를 수정하는 것과 다른 CL에 있어야 합니다. 리뷰어의 입장에서 변경사항을 이해하기 더 쉽습니다.

하지만 지역 변수의 이름 수정과 같은 작업은 기능 변경 또는 버그 수정 CL에 포함될 수 있습니다. 리팩토링이 너무 커서 현재 CL 리뷰가 너무 어려워진다면 리뷰어와 코드 작성자가 판단하면 됩니다.

빌드에 영향이 없어야 한다.

서로 의존하는 여러 개의 CL이 있는 경우, 각각 CL이 따로따로 반영된 후에도 전체 시스템이 계속 잘 동작해야 합니다. 그렇지 않으면 CL이 반영된 후에 동료 개발자의 빌드를 멈추게 할 수 있습니다.

더 이상 작게 만들 수 없어요.

때로는 CL을 작게 만들 수 없을 것 같아도 실제로는 그렇지 않습니다. 작은 CL로 만드는 것을 연습하는 개발자는 거의 항상 작은 CL로 나누는 방법을 잘 찾습니다. 큰 CL을 작성하기 전에 리팩토링을 위한 CL을 만들어 코드를 정리할 수 있는 기반을 고려해야 합니다. 팀원들과 이야기하여 작은 CL로 나누어 기능을 구현할 방법을 찾아보십시오. 그래도 방법이 없다면(이 경우는 매우 드물어야 함), 거대한 CL에 대하여 사전에 리뷰어의 동의를 구하고 리뷰 프로세스를 오랫동안 진행해야 합니다. 그리고 버그가 생기지 않도록 주의를 기울이고 더욱 더 철저하게 테스트 코드를 작성해야 합니다.

이어지는 글: 리뷰어의 의견을 다루는 방법