Hyeseong's Blog

최근 이런저런 커뮤니티서 코드리뷰에 대한 얘기가 부쩍 많아지는걸 느낍니다.

이상적인 코드리뷰가 무엇인가에 대해 누구나 저마다의 경험과 의견을 가지고 있을 것입니다. 이에 대해 이런저런 이야기가 나오는 걸 보니 일반적으로 동료와 협업할 때 또는 새로운 회사에 합류했을 때 어떤걸 기대하는지 엿볼 수 있어서 도움이 됩니다만, 아무래도 내용들을 100% 동의하기 어려운게 제가 또 마이너로 빠지고 있나 싶습니다.

저는 코드리뷰 과정을 전부 오픈소스 컨트리뷰션으로 배웠습니다. 일반적으로 회사 업무에 적용할만한 지침은 아닐지 모르지만 나름 경험으로부터 빚은 의견이 있긴 합니다. 일반론으로 내세우긴 좀 그렇지만 오픈소스 프로젝트 코드리뷰에 참여할 때 알아두면 좋을만한 마음가짐으로써 소개해보려고 합니다.

코드리뷰를 마주하는 자세

병합을 전제로 하는 코드리뷰는 프로젝트 메인테이닝 세션 즉, 코드 작성자가 메인테이너로서 참여하는 의식입니다

당신이 원래 커미터인지 아닌지는 상관없습니다. 리뷰 당사자라면 변경사항이 크던 작던, 기능 변경이던, 문서 추가던 아니면 단순 맞춤법 수정이던 간에 적어도 PR이 열려있는 동안은 메인테이너로서 행동해야 합니다.

예를 들어봅시다. 제가 PR을 하나 제출했습니다. 어지간한 git 워크플로우에서는 PR 하나를 기다려주느라 프로젝트 진행이 중단되지 않습니다. 동시에 다른 일이 일어납니다. 사람들과 함께 PR을 리뷰하는 동안, 업스트림에는 48개의 변경사항들이 추가로 커밋되었습니다. 이제 저든 리뷰어든 PR의 변경사항을 그 48개의 커밋으로 리베이스 할 수 있어야 합니다. 즉, 직접 변경한 부분이 아니더라도 동시에 변경된, 변경될 수 있는 의존성 맥락을 모두 쫒아야 함을 의미합니다.

그래서 프로젝트 자체나 변경의 맥락을 얼마나 알고있냐가 상당히 중요해집니다.

저는 사실 이미 구현전략이 결정되어 있고 협업자가 어떤 코드를 작성할지 100% 정확하게 예상할 수 있을 때는 코드리뷰의 필요성을 느끼지 못합니다. 솔직히 말해서 저는 어떤 모양일지 예측만 가능하다면 컨벤션도 별로 따지지 않습니다.

그렌라간 대사 - 널 믿는 나를 믿어

반대로 그렇지 않은 상황에서, 사전에 논의가 없던 요청이라면 대체로 리젝하거나 재작성 수순으로 향합니다. PR은 요청되면 반드시 병합되어야 하는 게 아니거든요. 리뷰 요청을 받는 순간 리뷰어에게도 책임이 넘어오기 때문에 맥락없이 코드만 보고 LGTM를 찍는건 쉽지 않습니다.

메인테이너로서, 그게 좋은 코드인지 아닌지는 별 관계가 없습니다. 모든 코드는 메인테이닝 부채거든요.

좋은 PR, 나쁜 PR

좋은 PR이냐 나쁜 PR이냐에 대해서, 프로젝트마다 나름의 가이드가 있을 수 있지만, 일반론일 뿐 절대적이지 않습니다. 결국 요청의 행방은 코드리뷰에서 합의를 통해 결정되기 때문입니다.

테스트 코드가 있으면 좋은 요청인가요? 코드도 멀쩡해보이고 테스트도 잘 통과하더라도 "올바르게 작성되었는지" 확인하는건 여전히 코드리뷰의 역할입니다. 이 PR은 e2e 테스트 시나리오를 추가하는, 일반적으로는 매우 좋은 내용입니다. 하지만 스펙에 명시된 시나리오와 비교해가며 코드를 검토하니 어설션 하나가 누락된 것을 발견할 수 있었습니다.

변경이 거대하면 나쁜 요청인가요? 어떤 변경은 필연적으로 커집니다. 이런걸 합당한 요청으로 만드는 것 또한 코드리뷰의 역할입니다. 이 PR은 기능 변경이 없는 리팩토링이지만 볼륨이 너무 거대했습니다. 하지만 내용이 맘에 들었습니다. 이걸 병합한다면 엄청 복잡했던 제어 코드가 선언적인 구조로 바뀌어서 앞으로의 메인테이닝 코스트를 크게 줄일 수 있었습니다. 그래서 이 변경이 너무 크다고 리젝하는 대신 PM, 개발자, 테스트 엔지니어를 포함한 13명의 리뷰어가 장장 3개월간 달라붙어 변경을 리뷰하고 피드백과 테스트 결과를 남겼습니다. 저는 피드백을 반영하면서 커밋 100여개를 수십번씩 업스트림에 리베이스 했습니다.

좋은 PR을 만드는 건 결국 좋은 리뷰입니다. 요청한 사람이던 받는 사람이던 리뷰하고 또 리뷰해야합니다.

그리고 충돌을 피하지 말고 리베이스와 테스트에 열과 성을 다하세요.

성장에는 책임이 따른다

코드리뷰를 통해 뭔가 배워서 성장하고, 코드 퀄리티가 올라가고, 팀웍이 좋아지고 할 수 있으나 모두 부가효과입니다.

프로젝트의 거버넌스 모델에 따라 느낌이 다 다르긴 하지만 본질적으로 병합을 전제로 하는 모든 코드리뷰는 메인테이너로서 참여하는 의식입니다.

코드 작성자와 리뷰어의 역할을 나누지 마세요. 코드 작성자는 당연히 1차적으로 리뷰어이며, 리뷰어는 반드시 작성된 코드에 대해 모두 이해해야 합니다. 코드리뷰 당사자는 모두 해당 코드 메인테이너라는 관점에서 역할이 같습니다.

프로젝트 기여과정에서 코드리뷰 참여는 성장에 도움이 됩니다만, 성장을 위한 도구가 아닙니다. 메인테이너 관점에서 생각하세요. 일방적으로 당신의 코드를 떠넘기지 마세요. 코드에 대한 책임을 지세요. (오래 지속되는 프로젝트에 참여한적이 있다면, 코드를 더하는게 능사가 아닌 건 알고 있을 겁니다)

결과적으로 성장하고자 한다면, 기여도를 높여야 할 것입니다. 그리고 큰 기여일수록 큰 책임이 따릅니다.

크던 작던 결과적으로 메인테이너와 관점이 일치하게되는 순간, PR은 쉽게 병합됩니다.

어쩌면 당신이 다음 메인테이너가 될지도 몰라요 :)

크리에이티브 커먼즈 라이선스