The Death of a Thousand Round Trips

• 코드를 읽으면서 사소한 것을 발견하자마자 리뷰 댓글로 지적하고, 그 즉시 읽기를 중단함
• 개발자는 성실하게 그 사소한 것을 고치고 수정된 패치를 제출함
• 리뷰어는 그 버전을 읽기 시작하고, 또 다른 사소한 것을 발견함. 첫 번째 리뷰에서 언급할 수 있었지만 그러지 않았음. 그 사소한 것을 지적하고 다시 읽기를 중단함
• 이를 개발자가 희망을 잃을 때까지 반복함

The Ransom Note

• 이 패치가 개발자에게 특별히 중요한 것 같음
• 그러나 리뷰어에게는 그렇게 중요하지 않음. 따라서 리뷰어는 권력 위치에 있음
• 개발자가 원하는 변경 사항을 리뷰어에게 중요한 추가 작업들이 완료될 때까지 인질로 잡아둘 수 있음
• 추가 작업은 실제로 동일한 커밋에 포함될 필요는 없지만 리뷰어에게는 중요한 작업임

The Double Team

• 하나의 패치, 두 명의 리뷰어
• 한 리뷰어가 변경을 요구할 때마다 개발자는 순순히 변경함. 그러면 다른 리뷰어가 불평할 차례가 됨
• 리뷰어들은 번갈아가며 서로 상충되는 요구사항을 제시함
• 하지만 항상 개발자를 향해 코멘트를 함. 리뷰 스레드에서 서로 직접 논쟁하는 것은 피함
• 개발자가 포기할 때까지 리뷰어들이 개발자를 앞뒤로 몇 번이나 튕겨낼 수 있는지 보는 것임

The Guessing Game

• 문제가 있고, 그 문제를 해결할 수 있는 다양한 방법이 있음
• 개발자는 한 가지 해결책을 선택하고 패치를 제출함
• 리뷰어는 그 특정 솔루션을 문제 해결 여부와 무관한 이유로 비판함
• 애매모호한 설계 원칙 위반이나 미래 개발 계획과의 비호환성 등을 이유로 삼음
• 비판을 충분히 모호하게 하면 개발자는 기존 패치를 어떻게 수정해야 비판을 해결할 수 있을지 알 수 없게 됨
• 개발자로서는 원래 문제를 해결하기 위해 완전히 다른 솔루션을 선택하고 대신 구현해보는 것이 최선의 방법임
• 그러면 리뷰어는 다시 마찬가지로 도움이 되지 않는 방식으로 그것을 거부함

The Priority Inversion

• 첫 번째 코드 리뷰에서는 사소하고 간단한 것들을 지적함
• 개발자가 그것들을 수정하기를 기다렸다가 중대한 문제를 제기함
• 패치의 상당 부분을 완전히 재작성해야 하는 근본적인 문제가 있음. 이는 이미 수행한 사소한 수정 작업의 많은 부분을 버려야 함을 의미함
• 누군가에게 많은 작업을 시키고 그것을 버리게 만드는 것만큼 "당신의 작업은 원하지 않으며, 당신의 시간은 소중하지 않다"는 메시지를 잘 전달하는 것은 없음
• 이것만으로도 개발자가 포기하기에 충분할 수 있음

The Late-Breaking Design Review

• 몇 주 또는 몇 달 동안 많은 별도 패치로 엄청나게 복잡한 작업이 진행되어 왔음
• 리뷰어는 그 전체 작업의 설계에 동의하지 않지만, 원래 논의에는 참여하지 않았거나 토론에서 패배한 측이었음
• 그런데 이제 리뷰어에게 그 작업의 중간에 있는 사소하지만 중요한 부분(예: 많은 개발자를 막고 있는 버그에 대한 쉬운 수정)을 검토해달라는 요청이 들어옴. 이것이 리뷰어에게는 기회임
• 리뷰어는 지금까지 수행된 작업의 전체 설계에 대한 정당성을 요구함
• 개발자가 전체 작업의 일부만 담당하고 있어서 답변을 모른다면, 그것은 리뷰어의 문제가 아님. 리뷰어가 만족할 때까지 "승인" 버튼을 누르지 않을 것임

The Catch-22

• 하나의 큰 패치라면 읽기가 너무 어려움. Self-Contained된 하위 조각으로 분할할 것을 요구함
• 반대로 작은 패치가 많다면 그 중 일부는 그 자체로는 의미가 없음. 다시 붙여넣을 것을 요구함
• 어떤 종류의 트레이드오프가 특정 경우에 관련이 있는 것처럼 보인다면 이를 활용해 불평할 이유를 찾을 수 있음
• 예를 들어 코드가 알아보기 쉽게 작성되었다면 성능이 좋지 않을 것이고, 잘 최적화되었다면 읽기 어렵고 유지 관리하기 어려울 것임

The Flip Flop

• 코드의 동일한 부분에 사람들이 일반적으로 수행하는 유형의 변경 사항이 있음
• 리뷰어는 이전에 이러한 변경 사항을 여러 번 검토한 적이 있음
• 또 다른 변경 사항이 들어옴. 작성자는 이전 변경 사항을 자세히 살펴보고 기존 패턴을 주의 깊게 따랐으며, 이 영역에 익숙해 보이는 리뷰어를 선택함
• 리뷰어는 이전에는 전혀 문제 삼지 않았던 변경 사항의 한 측면에 갑자기 이의를 제기함. 기존 패턴을 따르는 것만으로는 충분하지 않음
• 작성자가 이전에 동일한 변경 사항을 보여주면서 리뷰어의 비일관성을 지적하면 어떻게 될까?
• 리뷰어는 "맞다. 그것도 변경되어야 한다"라고 말함
• 리뷰어는 기존 사례를 모두 변경하겠다고 자원하지 않도록 주의해야 함. 운이 좋다면 개발자가 이를 기존 사례를 직접 변경하라는 지시로 해석하여 리뷰어가 많은 노력을 아낄 수 있음

그러나 진지하게 ...

• 지금까지 이 글은 농담이었음. 리뷰어들이 일부러 이런 나쁜 행동을 한다고 제안하려는 것도 아님
• 대부분의 설명은 리뷰어가 실제로 하는 일에 대한 과장이거나, 좌절한 패치 제출자가 느끼는 바에 대한 과장임
• 실제로 이런 일을 하지 말라는 것임!
• 라운드 트립을 최소화하도록 노력하고, 사소한 것들을 고르기 전에 (필요하다면) 중요한 재작성을 요청하며, 패치를 비판할 때는 어떤 버전을 수락할 것인지에 대한 건설적인 제안을 해야 함
• 전체 코드베이스에 대한 의견이 있다면 한 개발자의 패치를 검토하며 트집 잡기보다는 모든 개발자와 전반적인 논의를 해야 함
• 실수로 이런 일을 했다면, 자각해야함. 실수로 개발자의 삶을 더 어렵게 만들었다는 것을 인식하고, 사과하며, 더 도움이 되려고 노력해야 함
• 근본적인 문제는 권한의 남용임
• 한 개발자가 다른 개발자의 패치에 대한 코드 리뷰어가 되면 일시적인 권한이 생기게 됨. 리뷰어는 해당 패치가 커밋되는 것을 막을 수 있는 권한을 가짐
• 권한에는 책임이 따름. 의도된 목적으로, 필요한 만큼만 권한을 사용해야 함
• 대부분의 안티패턴(또는 풍자하는 온건한 행동)은 권한의 남용임. 리뷰어가 패치의 개선과는 무관하거나 반대되는 개인적 의제를 추구하기 위해 개발자에 대한 일시적인 권한을 지렛대로 사용하는 것임
• 개인적 의제는 안티패턴마다 다양함. 무관한 작업, 개인적 스타일 선호, 게으름, 변화에 대한 저항, 패치 제출자에 대한 개인적 혐오 등이 될 수 있음
• 패치 제출자가 과거에 코드 리뷰어였을 때 이런 나쁜 행동을 했다면 혐오가 정당화될 수도 있음. 그래서 코드 리뷰어의 권한을 신중하게 사용해야 함
• 발자들이 서로에 대한 복수의 악순환에 빠지면 소프트웨어 프로젝트는 파멸함

Gatekeeping 코드 리뷰

• 여기까지는 피어 간 코드 리뷰에 중점을 두었음
• 코드 리뷰어와 패치 제출자는 동료이며 서로 책임지거나 코드베이스에 대해 최종 결정권을 갖지 않음
• 그래서 코드 리뷰에서 얻는 권한은 일시적임
• 피어 리뷰 상황에서 코드 리뷰어와 작성자는 기본적으로 같은 목표를 가져야 함
• 어떤 기능을 넣을지, 승인 전 얼마나 다듬어야 하는지, 허용 가능한 코딩 스타일은 무엇인지에 대해 심각한 의견 불일치가 있다면 코드 리뷰 맥락 밖에서 다뤄야 함
• 하지만 다른 종류의 코드 리뷰 상황에서는 그렇지 않음. 특히 프로젝트 외부인이 요청받지 않은 패치를 보내는 경우엔 매우 다름
• 이런 시나리오는 일반적으로 프리 소프트웨어에서 발생함
• 전 세계 누구나 소스 코드를 수정할 수 있고 일부는 변경 사항을 다시 보내려고 시도하기 때문
• 하지만 다른 상황에서도 발생할 수 있음
• 사유(Proprietary) 코드를 개발하는 회사 내에서 한 팀의 개발자가 다른 팀의 프로젝트에 패치를 보내 운이 좋기를 바랄 수 있음
• 이런 상황에서는 패치 수신자가 애초에 해당 변경을 원하지 않거나 수행 방식에 완전히 동의하지 않아 패치를 전혀 받아들이지 않을 가능성이 훨씬 큼
• 이 경우에는 동료 리뷰어로서 부여받은 일시적 권한의 남용이 아니라 프로젝트 책임자로서 영구적 권한을 정당하게 행사하는 것임
• 내 소프트웨어 프로젝트의 방향은 내가 결정함
• 코드 리뷰어가 이런 '게이트키핑' 역할을 할 때는 패치가 기존 일반 설계 원칙이나 요구사항을 위반한다는 이유로 패치를 비판하는 것이 항상 잘못된 것은 아님
• 어쩌면 요구사항과 일치하는 방식으로 해당 문제를 해결하는 방법을 모를 수도 있음
• 사실 그것이 어려운 부분이고 내가 아직 동일한 변경을 하지 않은 유일한 이유일 수도 있음
• 그러나 게이트키핑 맥락에서도 설명 없이 'The Guessing Game'을 적용하는 것은 무례함
• 나는 일반적으로 개발자가 어려운 부분을 간과했음을 설명하려 노력하고, 내 자신도 답을 모르면 그렇다고 말함
• 물론 'The Death of a Thousand Round Trips' 같은 소극적이고 공격적인 방해에 대한 변명의 여지가 없음
• 정말로 패치를 코드에 전혀 넣고 싶지 않고 그 결정을 내릴 정당한 권한이 있는 게이트키핑 역할을 맡고 있다면, 제출자가 더 이상 시간을 낭비하지 않도록 말로 설명할 수 있음

Disclaimer

• 나는 수년 동안 내가 참여한(양쪽 모두에서) 코드 리뷰, 다른 사람들 사이에서 관찰한 코드 리뷰, 대화에서만 들은 코드 리뷰에서 이 기사를 위한 메모를 수집해 왔음
• 그래서 최근에 내 코드를 리뷰한 특정인을 겨냥한 것이 아님