좋은 코드리뷰에 대한 고찰
이 글은 코드리뷰 활동을 하면서 어떻게 하면 더 나은 코드리뷰활동을 이어갈 수 있을까 라는 생각을 정리한 글이다. 백명석님의 Code Review에 대하여라는 영상을 많이 참고하였습니다.
코드리뷰의 수요조사
아래 차트들은 2021년 프로그래머스에서 실시한 설문조사 결과이다.
흥미로운 조사결과는 많은 개발자들이 코드리뷰에 대한 필요성을 느끼고 팀에 코드리뷰를 도입을 하였거나 원하고 있음에도 불구하고 코드리뷰 도입을 후회하는 비율도 높다는 것이다.
코드리뷰라는 것이 이상적인 활동임에도 불구하고 실무에서는 이상적으로 수행하기가 쉽지 않거나 오히려 개발활동에 좋지 않은 영향을 미치는 경우가 발생하고 있음을 엿볼 수 있다.
그 이유는 무엇일까?
좋은 코드리뷰를 위한 전제조건
위와 같은 결과가 나오는 이유는 다양하겠지만 나는 좋지 않은 코드리뷰가 수행되는 가장 첫번째 이유가 환경이라고 생각한다. 코드리뷰를 할 상황이나 준비가 되어 있지 않음에도 불구하고 마냥 좋다라는 이유만으로 혹은 회사의 PR을 위해서 코드리뷰를 대책없이 도입하고 관리하지 않는다면 차라리 도입하지 않았을 때보다 생산성이나 팀원의 만족도는 떨어질 수 있다. 그럼 어떤 환경들이 조성되어야 하는지 알아보자.
도메인을 공유하고 있는 동료 개발자가 존재하는가?
도메인 지식을 공유하는 동료의 존재여부는 코드리뷰에서 아주 중요하다. 리뷰이가 작성한 PR내 코드들은 작업한 내용만 보여주기 때문에 해당 코드로 인해 발생하는 도메인의 영향범위를 해당 도메인 지식을 공유하고 있는 동료가 존재하지 않는다면 파악하기 쉽지 않다. 리뷰이가 아무리 친절하게 PR의 본문과 commit의 본문에 설명을 적어 놓더라도 말이다. 도메인 지식을 공유하는 리뷰어는 리뷰이가 미처 파악하지 못한 중대한 오류 발견하거나 좀더 효율적이거나 나은 구현방향을 제시해 줄 수 있다.
반대로 도메인 지식을 공유하는 동료가 아닌 리뷰이에게 코드리뷰를 받는다면 어떻게 될까? 물론 같은 포지션(예를 들어 백엔드끼리 혹은 프론트엔드 끼리)이라면 그나마 나은 리뷰를 받을 수 있겠지만 내가 생각하기에는 높은 확률로 오탈자나 사소한 디자인, 또는 잘못된 코드작성에 대해서는 리뷰를 받을 수 있겠지만 잘못된 설계 방향, 도메인에 대한 중대한 결함 등에 대한 리뷰를 받기가 쉽지 않다고 생각한다.
만약 동일한 포지션의 개발자가 존재하지 않은 경우는 더 나빠질 수 있다. 타 포지션의 개발자는 도메인 뿐만아니라 언어나 프레임워크에 대한 지식이 부족하기 때문에 대부분 Approve 머신(?)이 될 가능성이 크다. 이런 경우는 그냥 자기 자신이 리뷰를 승인하거나 PR 없이 코드를 병합하는 방법을 선택하는 것이 번거로움을 덜 수 있을 것이다.
팀내 개발자들이 코드리뷰에 대한 필요성을 인식하고 적극동참할 의지를 가지고 있는가?
팀원 전원이 코드리뷰에 대한 필요성을 인식하고 적극 참여할 의지를 가진 조직이냐 아니냐에 따라 그 팀의 리뷰문화는 상당히 다를 것이다.
전자라면 리뷰이가 PR을 작성할때 좀더 좋은 리뷰를 받을 수 있도록 하기 위한 노력을 할 것이고, 리뷰어 또한 적극적으로 리뷰에 동참하여 선순환을 이어갈 것이다.
하지만 후자라면 제대로된 코드리뷰 문화를 만드는 것에서 부터 삐걱대기 시작할 것이다. 코드리뷰를 하기 위해서는 어느정도 팀원들이 지켜야할 공통 규칙들이 생기기 마련이다. 팀원들이 코드리뷰에 대한 필요성을 느끼지 못한다면 이러한 규칙들은 불편한 제약사항일 뿐이라 생각하여 생산성을 떨어뜨린다는 생각까지 이어지게 되기도 한다. 또한 PR에 대한 리뷰가 바로바로 이루어 지지 않다보니 리뷰이는 리뷰어에게 PR의 리뷰를 종용하는 상황이 이어지고 개발자끼리의 감정의 골이 점점 커지거나 혹은 리뷰를 그냥 의무적인 행위로 간주하여 승인을 기계적으로 하는 상황이 생기기도 한다.
좋은 코드리뷰 문화를 가지려면 팀원들이 왜 코드리뷰를 해야하고 어떻게 참가해야 할지를 인지시켜야 한다. 아마 시간이 지나면서 리뷰에 대한 회의감을 느끼거나 코드리뷰로 인해 불화가 발생하는 팀원이 생길 수도 있다. 그러므로 자주, 종종 코드리뷰를 우리가 왜 하고 있는지 더 잘하기 위한 방법을 상기키셔야 한다.
리뷰에 대한 팀내 정책이 확립되었는가?
코드리뷰는 회사마다 팀마다 그 정책이 다를 수 있다. 팀의 상황에 맞게 점점 변화하고 보완해 가기 때문일 텐데, 좋은 리뷰문화를 지속적으로 이어가기 위해서 팀내 코드리뷰에 대한 정책을 확립해둘 필요성이 있다. 정책을 확립할 때 주의해야할 부분은 팀원 대다수의 동의를 얻어서 정하는 것이고, 너무 디테일한 정책은 지양해야 한다는 것이다. 팀의 상황에 따라 처음에 정했던 정책이 변화하기는 하겠지만 자주 변경되는 정책은 오히려 팀원들의 혼란을 초래할 것이다. 팀원 대다수가 동의하지 않거나 너무 디테일한 정책은 변경되기 쉽다. 모호한 부분은 명확하게 하고 디테일한 부분은 상황에 맞게 리뷰어나 리뷰이가 융통성(?)있게 풀어가면 좋겠다.
코드리뷰 시 주의할 점
지금까지 코드리뷰를 도입하기 전 전제조건에 대해서 이야기 해보았다. 다음으로는 좀더 자세하게 들어가서 코드리뷰를 하는 상황에서 어떤 부분을 주의하면 더 나은 코드리뷰를 할 수 있을지 이야기 해보자.
코드리뷰의 목적을 인지하자
코드리뷰란 아래와 같이 정의할 수 있다. Wikipedia 참고
코드 리뷰(Code Review)는 소스 코드의 일부를 주로 보고 읽음으로써 한 명 또는 여러 명이 프로그램을 점검하는 소프트웨어 품질 보증 활동이다. 사용자 중 적어도 한 명은 코드의 작성자가 아니어야 합니다. 작성자를 제외하고 검사를 수행하는 사람을 “리뷰어(Reviewer)”라고 합니다.
코드리뷰는 코드를 지적하기 위한 활동이 아니다. 품질을 높이기 위한 노력이다. 하지만 코드리뷰의 시스템 특성상 과제검사(?)와 같이 느껴지는 부분은 어쩔 수 없다. 리뷰이는 자신도 모르게 PR을 작성하면 리뷰에 대해 걱정이 생길 수 밖에 없고, 리뷰어도 PR에 대한 리뷰를 참가할 때 리뷰를 꼭 작성해 주어야한다는 강박관념에 빠지기 쉽다. 리뷰이와 리뷰어 모두 우리가 왜 코드리뷰를 도입하고 시간을 투자하여 이 활동을 하고 있는지 자주 상기할 필요가 있다.
코드리뷰 참가자들을 배려하자
코드리뷰의 목적을 상기했다면 다음은 배려이다. 코드리뷰에서 가장 중요한 부분이라고 생각하고 배려가 빠지면 좋은 코드리뷰라는 것이 성립할 수 없다고 생각한다. 결국 코드리뷰는 사람이 하는 활동이다. 서로의 배려없이 리뷰활동이 이어지다보면 감정의 골은 자신도 모르게 깊어질 수 있다. 배려라는 것은 거창한 것이 아니다. 아래에 제시될 항목들도 배려를 전제로 하는 것이긴 하지만 아주 사소하게 내시간을 좀더 투자해서 좋은 리뷰를 남기기 위해 노력하거나 PR을 작성할 때 리뷰이가 좀더 이해하기 쉽게 풀어서 설명한다던지, 리뷰 시 좀더 부드러운 단어를 선택한다던지 등이 될 수 있다.
코드의 양과 복잡성을 생각해서 PR을 크기를 줄이자
이런 말이 있다.
PR의 크기가 크면 리뷰가 없고, PR의 크기가 작으면 리뷰가 많다.
PR의 크기는 단순히 코드의 량으로만 판단해선 안된다. 복잡하지 않고 단순 반복되는 코드가 많은 경우라면 PR의 크기는 크지 않을 수도 있다. 결국 PR의 크기란 코드의 량과 함께 복잡성도 함께 고려되어야 한다.
복잡성은 문맥(Context) 또는 목적과도 관련성이 있는데 예를 들어 PR에 새롭게 추가할 기능과 리펙토링 코드가 뒤섞여 있다면 그 PR은 나누어서 작성하는 것이 리뷰어가 리뷰를 하기가 훨씬 좋다. 리펙토링은 리뷰어가 좀더 가볍게 리뷰를 할 수 있을 테고 새로운 기능의 추가에 좀더 집중에서 리뷰에 참가할 수 있기 때문에 좋은 리뷰가 나올 가능성이 높다.
코드 == 나 라는 인식을 버리자
나도 이 인식을 버리는 것이 제일 어려운거 같다. 리뷰이는 리뷰어가 지적한 코드가 나에 대한 지적으로 인지하는 경우가 많다. 상급자로 갈수록 그렇게 생각할 가능성이 더 높아진다고 생각한다. 하지만 이러한 생각을 접어둘 필요가 있다. 리뷰어는 단순히 지금 작성된 코드에 대한 리뷰를 하는 것일 뿐인데 리뷰이가 민감하게 반응한다면 리뷰어는 리뷰이의 눈치를 보는 상황이 생기게 된다. 이런 상황이 지속되면 리뷰를 통해 좋은 방향으로 제품이 개발될 수 있음에도 불구하고 리뷰어가 리뷰를 꺼림으로써 개선의 기회를 놓칠 수 있다.
자신의 코드가 자신을 나타낸다는 생각을 버리자. 그렇다면 리뷰어의 리뷰에 객관적으로 피드백을 할 수 있을 것이다.
코드 스타일은 되도록 자동화된 formatting tool, lint로 해결하자
팀내 코드 스타일 정책을 가진다면 여러 팀원이 코드를 작성하더라도 한사람이 작업한 것과 같은 효율적인 가독성을 꾀할 수 있다. 팀원들은 특정한 코드의 위치가 어디에 존재할지 예상할 수 있고, 네이밍 규칙을 통해서 해당 코드를 세세하게 보지 않더라도 코드의 동작을 예상할 수도 있다.
하지만 너무 엄격한 코드 스타일은 오히려 개발에 방해가 될 수 있다. 팀원 모두 모든 스타일을 일치시킬 수 없기에 지나친 스타일 정책을 가지고 스타일을 강조하는 것은 바람직하지 않다고 생각한다. 그래서 코드 리뷰에서 세세한 코드 스타일을 지켜줄 것을 강제하다보면 리뷰이와 리뷰어 모두 정작 중요한 포인트를 놓칠 수도 있다.
코드 스타일은 되도록이면 자동화된 formatting tool과 lint를 사용하자. IDEA툴 설정이든 Git hook을 통한 Lint 검사든 어떤 방법이어도 좋다. 이런 설정은 한번 설정만 잘해 둔다면 더이상 리뷰이와 리뷰어가 코드 스타일에 신경쓰지 않아도 된다. 그외에 자동화 할 수 없는 것들은 nice to로 두어야지 코드 리뷰에 스타일로 인한 스트레스를 가중 시킬 필요는 없다고 생각한다.
고수준에서 저수준으로 리뷰하라
고심해서 작성한 코드를 PR을 작성한 후 리뷰 요청을 하였는데, 수십개의 리뷰가 달린다고 생각해보자. 아마도 리뷰이는 당황스러울 것이다. 이런 상황은 팀에 합류한지 얼마 되지 않은 팀원인 경우 더 높은 확율로 마주칠 가능성이 높다. 팀내 코딩 정책도 익숙하지 않을 수도 있고, 긴장감 또는 기존의 습관 등등 아직 팀의 문화에 적응하기 전이니 리뷰어들은 더더욱 꼼꼼하게 코드리뷰를 수행할 것이고 수정할 만한 것들이 아주많이 보일 것이다.
고수준 부터 1차 리뷰를 한 후 2차로 저수준 코드를 리뷰하는 방법을 채택하면 리뷰이가 처음부터 어마어마한 코드 리뷰 수를 마주치게되면서 당황하는 상황을 다소 피할 수 있다. 고수준은 인터페이스 설계나 구조 등등을 본다고 생각하면 될것이고 저수준은 오탈자나 주석, 세부적인 코드 스타일등을 말하는 것이다.
예제 코드를 제공하라 단, 지나치지 않도록
리뷰에서 단순히 작성된 코드를 수정해달라는 코멘트를 남기는 것보다는 예제코드를 통해 제안하는 방법은 리뷰이에게 아주 친절한 리뷰일 수 있다. 예시 코드만큼 리뷰어의 의도를 명확하게 표현하는 방법은 없기 때문이다.
하지만 예제코드를 제안하는 것도 과하면 오히려 관대한게 아니라 억압일 수 있다. 그래서 예제코드의 제안은 중요한 부분만 작성하면서 디테일한 부분은 리뷰이가 알아서 할 수 있도록 최소한으로 유지하는게 좋다고 생각한다. 그리고 예제코드의 제안 수를 제한함으로써 리뷰어가 무시받는다는 느낌을 받지 않도록 주의할 필요가 있다.
객관적으로 피드백하라
별론거 같아요., 이때까지 이렇게 해왔어요.와 같은 리뷰와 피드백은 좋지 않다. 명확하고 객관적인 이유가 없는 리뷰는 차라리 달지 않는 것이 리뷰어와 리뷰이의 에너지 소비를 줄일 수 있다.
누구나 납득할만한 객관적이고 일반적인 원칙을 제시하자. 설득력은 거기서부터 온다. 물론 개발자의 취향이 반영된 리뷰가 있을 수 있다. 그렇다면 적어도 그렇게 생각하게 된 이유와 그 이유를 뒷받침하는 레퍼런스를 적어주는 것이 좋다고 생각한다.
팀원간의 리뷰에 대한 룰을 정하자
뱅크샐러드의 Pn룰을 처음 접했을 때 개인적으로 매우 참신하고 효율적인 규칙이라 느껴졌다. 리뷰어들은 리뷰를 남길때 자신이 남기는 리뷰의 내용들이 종합적으로 Request Change인지 Comment인지 Approve인지를 선택하게 된다. Pn룰은 리뷰를 남기는 시점에 Request Change인지 Comment인지 Approve인지를 판단할 수 있는 기준을 제시하므로 리뷰어가 마지막에 종합해야할 고민거리를 덜어줄 수 있다.
그리고 리뷰이는 리뷰어가 작성한 리뷰가 어느정도의 요청사항인지 단번에 인지할 수 있기 때문에 중요도에 다라 반영여부를 선택할 수 있으므로 커뮤니케이션 비용을 많이 줄일 수 있다.
이와 같이 팀원간에 리뷰를 좀더 효율적으로 할 수 있는 공통된 룰을 정해놓으면 좀더 나은 리뷰문화를 가질 수 있을 것이다.
Request Change는 치명적인 오류를 가지고 있거나 누가 봐도 나쁜 디자인인 경우에 사용하자
선택적 제안과 치명적인 오류 또는 객관적으로 좋지 않은 디자인인 경우는 다르다. 자신이 좋다고 생각하는 구현방향으로 인해 리뷰이의 구현을 강제하지 말자. 만약 객관적으로 좋지 않은 설계인 경우에는 그 이유를 친절히 설명함으로써 리뷰이와의 커뮤니케이션 비용을 줄이는 것이 좋겠다.
또한 Request Change를 받은 리뷰이는 해당 리뷰에 대해 적극적인 피드백을 하며 의견에 동의하지 않는 경우 리뷰어와 마찬가지로 일반적이고 객관적인 이유를 기반하여 설득을 하여야 한다.
리뷰 라운드는 최대 하루로 정하는 것이 좋다
리뷰가 길어지면 다음 작업이 쌓여서 생산성에 영향을 미칠 가능성이 높다. (PR이 크다면 이 문제는 더욱 커질 수 있다.) 그러니 리뷰어나 리뷰이는 PR이 작성되고 리뷰요청을 받는다면 적극적으로 리뷰를 해주는 것이 서로에게 좋다.
물론 다른 작업으로 인해 Context switching 비용을 우려할 수 있다. 하지만 작업의 범위를 잘게 쪼개서 한다면 선행 작업을 마무리한 후 다음 작업을 진행할 때 PR을 리뷰하는 등의 방법을 도입한다면 Context switching 비용에 대한 고민을 다소 해결할 수 있으리라 생각한다.
그리고 만약 리뷰 중 교착상태에 빠졌다면 구두로 얘기를 해서라도 빠르게 문제를 해결하려는 노력을 기울어야 한다. 교착 상태 얘기가 나온김에 코드리뷰의 목표에 대해서 잠깐 얘기를 해보고자 한다. 코드리뷰의 목표는 코드레벨을 A로 만드는 것이 아니다. D등급의 PR을 받으면 C나 B등급을 받도록 하는 것이 우리가 코드리뷰를 하는 목표라고 생각하면 좋겠다. 그리고 만약 승인을 보류해야 하는 경우는 기능이 틀렸거나 너무 복잡해서 정신이 없는 상태이다.
그래서 사소한 이슈만 있다면 흔쾌히 승인하자. 서로의 입장차를 좁히지 못해서 교착상태에 빠지곤 하는데 누가 보아도 문제인 코드는 교착상태에 쉽게 빠지지 않는다. 자신의 의견은 한번 또는 최대 2번까지 관철 시켜보고 입장차가 좁혀지지 않으면 쿨하게 상대방의 의견을 받아들여보자. 시간이 지나면 의외로 그 문제는 아무것도 아닐 수 있다. 중요한건 팀원끼리의 팀워크와 효율적인 업무를 이어가는 것이다. 처음 이야기했던 우리가 리뷰를 하는 이유를 상기해보자.
반복적인 패턴에 대해서 피드백을 제한하라
동일 패턴에 대한 리뷰는 2~3번정도만 언급하거나 하나의 리뷰로 나머지도 확인해달라는 요청을 해보자. 이는 고수준에서 저수준으로 리뷰하라에서 언급했던 코드리뷰의 수를 줄임으로써 리뷰이가 당황하는 것을 방지하는 방법중 하나이다.
리뷰의 범위를 존중하라
리뷰를 하다보면 근처 코드에 수정했으면 하는 것들이 보일 수 있다. 하지만 그 마음을 꾹 참고 리뷰할 범위 내에서만 리뷰를 하는 것이 좋다. 근처에 있는 코드까지 리뷰를 하기 시작하면 PR에 작성된 코드이외의 내용에 대해서 토론을 하는 상황이 생길 수 있다.
여기서 예외 되는 경우도 있는데, 바로 해당 PR에 작성된 코드로 인해서 둘러싼 코드에 영향을 미치게 되는 경우이다. 이런 경우에 PR의 범위를 벗어낫지만 리뷰를 할 수 밖에 없는 이유와 함께 리뷰를 작성한다면 좋을 것 같다. 어떤 영향을 미칠 수 있는지 친절한 설명이 있다면 더할 나위 없다.
좋은 코드리뷰를 위하여
우리는 어떻게하면 일을 잘할 수 있고, 빨리 할 수 있고, 올바르게 할 수 있는지 고민한다. 그러한 고민 중 하나의 해결방법이 코드리뷰라 생각한다.
하지만 코드리뷰가 우리가 하는 일을 방해한다고 생각되게 하면 안된다. 리뷰이가 PR 작성 후 리뷰를 무서워 한다면 우리의 코드리뷰 문화에 문제가 있지않은지 고민해야한다.
Agree to disagree, Google
저수준 코드를 무심코 승인하면 품질 좋은 SW를 만들수 없고, 코드리뷰로 인해 동료와 다퉈서 함께 일하지 않는다면 고수준의 품질을 얻을 수 없다.
개발도 결국 사람이 하는거다 배려하고 칭찬하자.
https://yungis.dev/github/thought-about-good-pr/