https://www.chiark.greenend.org.uk/~sgtatham/quasiblog/code-review-antipatt

코드리뷰는 리뷰어와 개발자 모두 도움이 되는 측면이 있다. 하지만 리뷰어가 잘못하면 코드 개선에 매우 큰 장애물이 될 수도 있다. 개발자를 짜증나게하고 개선을 포기하게 할수도 있는 것이다.

여기서는 이를 예방하기 위해 코드리뷰 안티패턴을 반어적으로 소개한다

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

큰 패치가 있을때 알아보기 힘드니 작게 나눠달라고 요구하라. 그런데 만약 작은 패치가 여러개 있으면 하나하나는 의미가 없으니 크게 하나로 묶어달라고 하라.

이런 것처럼 trade-off 관계의 것을 지적하라 (ex : 가독성이 좋다면 성능을 지적, 성능이 좋다면 가독성을 지적)

trade-off 관계의 것을 지적할때는 조심하라

The Flip Flop

같은 부분을 많은 사람이 수정하는 경우가 있다. (리스트 등). 개발자가 이전 관습을 준수하여 수정을 하였고, 이제까지 그러한 관습은 Approve 되었다. 그런데 갑자기 이전에 문제 삼지 않았던 부분을 문제삼아라.

만약 개발자가 이제까지와의 불일치를 지적한다면, 이제까지의 코드도 수정해야한다고 말하라. 물론 너가 고치겠다고 자원하지마라.

But seriously, folks

물론 위 예시는 농담이고 반대로 얘기한 것이다.

Authority

여기서 중요한 점은 코드 리뷰어가 개발자에 대해 권위를 가지게 된다는 점이다. 리뷰어는 코드를 승인하지 않을 권위가 생기게 된다.

권위는 필요한만큼만 사용되어야한다. 이러한 경우 코드를 가능한 좋게 만들어야하는 걸 말하는데, 이 좋다는 개발팀 전체가 동의하는 정의를 따른다.

따라서 이러한 안티패턴들은 권위를 남용하여 발생한다. 리뷰어는 코드의 장점을 무시하고 개인적인 다른 의제를 꺼내어 코드를 승인하지 않을 수 있다. 이러한 개인적 의제는 안티패턴에 따라 다양하다.

이렇게되면 다음에 리뷰어가 개발자가 되고, 그 개발자가 리뷰어가 되었을때 복수가 시작될 수 있다. 이러한 복수의 연쇄가 되면 프로젝트는 망한다.

Gatekeeping code review

따라서 필자는 peer code review 에 초점을 맞춘다. 개발팀 내부에서 서로 리뷰어가 되는 것이다. 리뷰어와 개발자는 동료이고, 이 관계는 언제든 바뀔 수 있다. peer code review 에서는 리뷰어와 개발자가 같은 목표를 가질 수 있다.

그런데 만약 개발팀 외부의 코드를 리뷰할때는 상황이 다르다. 리뷰어는 그 코드를 승인하지 않을 가능성이 훨씬 크다. 이러한 경우는 권위를 남용한 것이 아니다. 우리 프로젝트는 우리팀에서 결정할 권한이 있기 때문이다. 이처럼 리뷰어가 게이트키퍼 역할을 할 경우, 리뷰어는 근본적인 디자인패턴을 지적할 수 있다.

하지만 이때도 아무런 설명없이 그저 거부만하는 것은 좋지 않다 (Guessing Game 처럼). 어떻게 수정하면 좋을지 설명하고 잘 모르겠으면 모르겠다고 말하는 것이 좋다. 또한 역시 Thousand Round Trip 예시는 하면 안된다.