Michael Lynch의 How to Make Your Code Reviewer Fall in Love with You을 번역한 글 입니다.
코드 리뷰에 대한 조언 중에서 리뷰어의 입장으로 쓰인 것은 이미 많이 접해보셨을 겁니다. 그러나 사실은, 코드 리뷰를 요청하는 사람의 역할도 리뷰어의 역할만큼이나 중요합니다. 많은 분이 코드 리뷰 요청에 미숙한 것은 이 '리뷰이' 역할에 대한 가이드가 별로 없는 탓이라고 생각합니다.
이 글이 바로 그 가이드입니다. 이 글을 읽은 후엔, 당신의 코드 리뷰 요청이 너무 능숙해진 나머지 리뷰어가 당신에게 사랑에 빠지는 불상사 아닌 불상사가 일어날 겁니다.
근데요... 저는 사랑에 빠지게 할 의도까지는 없는데요😅
사랑받으실 겁니다. 익숙해지세요. '죽을 때 후회하는 N가지' 부류의 책에서, 살면서 사랑을 너무 많이 받아서 한스럽다는 말은 아직까지 제가 읽어 본 적이 없습니다.
#왜 코드 리뷰를 개선해야 할까요?
코드 리뷰 스킬을 개선하는 것은 일차적으로 리뷰어에게, 그리고 당신의 팀에게 도움이 됩니다. 그러나 가장 많은 덕을 볼 사람은 바로, 당신입니다!
하나, 더 빨리 배울 수 있습니다. changelist를 제대로 작성함으로써, 내가 성장하기 위해 도움이 필요한 바로 그 지점에 리뷰어가 집중하도록 유도할 수 있습니다. 건설적인 비판에 유연한 대응을 보여준다면, 향후 도움 되는 피드백을 더 많이 받을 가능성 또한 열립니다.
둘, 덩달아 다른 사람들까지 좋아집니다. 내가 잘하면, 내 코드 리뷰 요청 방식이 동료들에게 일종의 가이드가 될 수 있습니다. 물 흐르듯 편안한 코드 리뷰 경험을 한 팀원들은 자연스럽게 좋은 방식을 따라오게 될 겁니다.
셋, 팀 갈등을 줄일 수 있습니다. 코드 리뷰가 팀 내의 마찰의 원인이 되는 경우가 잦습니다. 의식적이고 조심스러운 접근을 통해서 이를 어느 정도 예방할 수 있습니다.
#Golden Rule: 리뷰어의 시간은 금금금💎
뻔한 조언이라 생각할지 모르겠습니다만, 저는 실제로 리뷰어를 개인 QA 테크니션으로 취급하는 개발자들을 종종 봅니다. 이 개발자들은 PR 전에 에러가 있나 한 번 더 들여다본다던가, chanelist를 작성하던가 하는 식의 '리뷰어에 대한 의식적인 배려'가 전혀 없더군요.
나의 동료들은 매일 하루치의 유한한 집중력을 가지고 출근합니다. 이 중의 일부를 내 코드를 봐주는 데에 쓴다면, 그들 자신의 일에 쓸 시간은 그만큼 줄어듭니다. 리뷰이로써 리뷰어의 시간을 최대한 아껴줘야 한다는 것은 너무나 당연하지 않은가요?
코드 리뷰는 상호 신뢰가 있을 때 효과가 극적으로 상승합니다. 상대방이 피드백을 진지하게 받아들일 거라는 믿음이 있어야만 신난 리뷰어가 더 열심히 피드백을 주게 된다는 거죠. 리뷰어를 '걸림돌'과 같은 것으로 취급하는 순간, 리뷰어가 나에게 되돌려 줄 가치는 급격히 줄어들 겁니다.
#차례
- 셀프 코드 리뷰를 먼저 해보기
- Changelist를 명확하게 작성하기
- 간단한 일은 자동화 하기
- 질문에 대한 답은 코드 그 자체로
- 수정사항의 범위 제한하기
- 기능 수정사항과 나머지 수정사항을 구분하기
- 수정사항 쪼개기
- 비판에 관대하기
- 리뷰어가 틀렸을 때 참을성 갖기
- 주도권을 확실하게 주고 받기
- 빠진 정보를 똑똑하게 알아내는 요령
- 비등비등할 때는 리뷰어의 편을 들어주기
- 리뷰와 리뷰 사이의 지연시간을 최소화
#1. 셀프 코드 리뷰를 먼저 해보기
작성한 코드를 팀원에게 보여주기 전에 한 번만 시간을 내어 스스로 읽어보세요. 단순히 실수가 있나 검토해보라는 의미뿐만이 아니에요. 그보단 내가 이 코드를 처음 읽는다면 어떨까 생각해보세요. 뭔가 부자연스럽거나 이상한 부분이 있지는 않은가요?
코드를 작성하고 나서 셀프 코드 리뷰를 하기 전까지 잠시 시간 간격을 두는 것도 요령입니다. 퇴근할 때 즈음 그날의 일을 마무리하며 호다닥 리뷰 요청 보내 놓으시는 분들이 있을 텐데, 사실 이때가 부주의한 실수가 생겨나기 가장 좋은 타이밍입니다. 그 코드, 다음날 아침까지만 잠시 묵혀두었다가 새로운 날의 새로운 눈으로 한 번 훑어보고 난 후 팀원에게 보내는 걸 추천합니다.
리뷰어의 환경을 최대한 따라 해 보세요. 리뷰어와 동일한 diff 툴을 사용하고요. 작업하던 코드 에디터를 벗어나서 diff 툴의 UI로 코드를 한 번 보는 것만으로도 단순한 실수는 쉽게 찾을 수 있을 거예요.
완벽하지 않아도 괜찮습니다. 디버깅 코드를 지우지 않은 채로 리뷰 요청을 보내거나, 엉뚱한 파일을 포함해 커밋하는 등의 실수는 누구나 합니다. 다만 이런 일이 생겼을 때 자책보다는, 일단은 유심히 기억해두셔야 합니다. 내 실수의 패턴을 주의 깊게 관찰했다가 이를 보완할 수 있는 시스템을 만드는 겁니다. 이런 작은 실수들이 반복된다면 상대 리뷰어는 '이 사람은 내 시간을 존중해주지 않는다'는 인상을 받을 겁니다.
#2. Changelist를 명확하게 작성하기
전 직장에서 개발자 멘토링 프로그램을 통해서 한 시니어 엔지니어 분을 주기적으로 만나게 되었는데요. 첫 미팅에 제가 작성한 설계 문서를 가져오라고 말씀하시더군요. 문서를 전달하면서 프로젝트 내용을 소개하고, 이 프로젝트와 연결해서 팀의 목표를 어떻게 세웠는지도 같이 설명드렸습니다. 그랬더니 그 시니어 엔지니어는 인상을 찌푸리면서, "방금 설명한 내용은 디자인 문서의 맨 첫 번째 페이지에 넣으셨어야죠."라고 무뚝뚝하게 얘기했습니다.
그분 말이 맞습니다. 저는 팀원들이 읽을 것만을 상상하며 문서를 작성했습니다. 이외의 독자들에 대해서는 전혀 고려하지 않았죠. 사실은 협력 팀, 멘토들, 그리고 승진 위원회(역주: 구글의 promotion commitee를 말합니다)까지 고려했어야 하는데 말이죠. 이 대화 이후로 저는 제 일에 대해 설명할 때면 언제나, 어떤 방식으로 설명해야 맥락까지 함께 전달할 수 있을까를 고민하게 되었습니다.
코드 리뷰의 changelist를 작성할 때는 필요한 모든 배경지식을 간략하게 포함해야 합니다. 특정 리뷰어를 염두에 두고 리뷰 요청 내용을 작성하는 경우에도, 그 사람이 배경지식을 얼마간 알고 있다고 함부로 가정해 버리면 안 됩니다. 게다가 다른 팀원들이 그 내용에 대해 읽을 필요가 생겼을 때, 수정사항의 의도를 명확히 작성해두어야 나중에도 히스토리 파악하기가 용이합니다.
이 수정사항을 통해 이루고자 하는 것이 무엇인지, 그리고 이 수정사항이 큰 맥락에서 왜 필요한지를 포함해야 좋은 chagelist라고 부를 수 있겠습니다.
Changelist를 잘 작성하는 방법에 대해 궁금하시다면, 이 글들을 읽어보세요.
- “How to Write a Git Commit Message” by Chris Beams
- “My favourite Git commit” by David Thompson
#3. 간단한 일은 자동화하기
괄호가 잘못된 라인에 있다거나, 테스트가 깨졌다거나 할 때 리뷰어를 통해 듣고 그제야 알아차리고 있지는 않은가요? 그렇다면 소중한 시간을 낭비하고 계신 겁니다.
자동화된 테스트는 팀이 항상 거치는 업무 절차의 일부가 되어야 합니다. 코드 리뷰는 CI 환경에서 모든 자동화 테스트를 통과하고 난 후에 시작하는 겁니다.
만약 팀원들이 도와주지 않고 팀 차원에서 관심도 크게 없는 상황이라면, 스스로 자동화된 환경을 만들어보세요. 매번 커밋할 때 특정 동작을 추가하고 싶다면 Git pre-commit hook을 추가하고, 코딩 컨벤션에 대한 번거로움을 덜고 싶다면 lintter와 formatter를 개발 환경에 추가해보세요.
#4. 질문에 대한 답은 코드 그 자체로
사진에서 잘못된 점이 무엇일까요?
작성자는 코멘트를 통해서 함수를 작성한 의도에 대해 저에게 설명해주었지만, 제 다음에 코드를 읽는 다른 사람은 어떨까요? 변경 이력이 포함된 PR을 찾아가서 코드 리뷰 코멘트를 다 읽어봐야 하는 걸까요? 이것보다 더 최악인 것은 작성자가 제 자리로 와서 구두로 이 내용을 설명해주는 것입니다. 제 집중은 집중대로 흐트러지고, 이제 그 내용은 기록되어 있지도 않게 되었으니까요.
코드 리뷰 중에 코드의 동작에 대한 질문이 나온다면, 그걸 설명해주는 것이 능사가 아닙니다. 설명하려면, 모두에게 설명해야 할 테니까요.
이런 질문을 받았을 때 가장 좋은 대처 방법은, 코드를 리팩터링 해서 질문의 여지를 없애버리는 겁니다. 이름을 바꾸거나 구조를 변경해서, 코드 그 자체로 더 명확한 표현을 담을 수는 없는지 고민해보자는 거죠. 주석을 다는 것도 하나의 방법이긴 하지만, 그보다는 그 자체로 문서의 역할을 하는 코드를 작성하는 것을 권해드리고 싶습니다.
#5. 수정사항의 범위 제한하기
코드 리뷰의 흔한 안티 패턴으로 수정사항의 경계를 가뿐히 무시하는 경우가 있습니다. 로직과 관련된 버그를 수정하다가 UI에 작은 흠을 발견한 개발자는 생각합니다.
이왕 이 파일 수정하는 김에, 이것도 같이 고칠까?
네, 뒤죽박죽 엉망진창으로 가는 헬게이트가 지금 막 열리려 하고 있네요. 이제 리뷰어는 어떤 수정사항이 A 목적을 위한 것이고 어떤 수정사항이 B 목적을 위한 것인지를 구분하며 코드를 읽어야 합니다.
단 하나의 기능(Do One Thing)만을 포함한 changelist가 가장 좋습니다. 수정사항이 작고 간단할수록 리뷰어가 모든 컨텍스트를 기억해가며 리뷰하기 좋습니다. 또한 연관성이 작은 수정사항들을 서로 분리하면 각각의 리뷰를 병렬로 진행할 수 있기 때문에 리뷰 라운드를 주고받는 시간을 아낄 수 있습니다.
#6. 기능 수정사항과 나머지 수정사항을 구분하기
리뷰 단위가 되는 수정사항의 범위를 최소한으로 줄여나가다 보면, 결국은 기능 수정사항과 이외 나머지 수정사항을 구분하는 데에 이르게 됩니다.
코드 리뷰 경험이 별로 없는 분들이 이 규칙을 간과하기 쉽습니다. 예를 들어서 실제 코드를 수정한 내용은 두 줄인데, 코드 에디터가 전체 파일을 reformat 해버리는 겁니다. 이렇게 되면 리뷰어가 의미 있는 수정사항에 대해 인지하기 힘들어집니다. 몇백 줄의 인덴트 변경사항의 틈에서 두 줄의 기능 수정사항이 눈에 띌 리가 없겠죠.
아래와 같은 인덴트의 습격 속에서, 기능 수정사항을 잡아 낼 수 있으신가요?
저는 이런 식으로 수정사항을 뒤죽박죽 섞어놓는 것은 리뷰어에게 무례한 행동이라고 봅니다. 인덴트 수정사 항만 담고 있는 코드는 리뷰하기 쉽습니다. 두 줄의 기능 수정사항도 리뷰하기 쉽습니다. 반면 인덴트 수정사항의 홍수 속에 파묻힌 두 줄의 기능 수정사항은 리뷰하기 정말 힘듭니다.
리팩터링을 하다가 수정사항이 섞이는 경우도 꽤 있습니다. 저는 제 팀원이 코드 리팩터링 하는 게 정말 좋은데요, 리팩터링 하다가 코드의 행동까지 같이 변경하는 건 정말 싫습니다.
아래의 코드는 동작을 변경한 것이 딱 한 군데에 있는데, 리팩터링 때문에 눈에 전혀 안 들어옵니다.
만약에 리팩터링과 동작의 변경이 둘 다 필요한 상황이라면, 두세 개의 작업으로 쪼개세요.
- 기존 동작에 부합하는 테스트를 (기존에 없다면) 작성한다.
- 테스트 코드를 유지한 채로 프로덕션 코드를 리팩터링 한다.
- 프로덕션 코드의 동작을 변경하고, 테스트 코드도 그에 맞게 수정한다.
자동화된 테스트를 2번에서 그대로 유지함으로써, 동작 변경 없이 리팩터링만 했다는 사실을 리뷰어에게 증명할 수 있습니다. 3번에서 리뷰어는 리팩터링 수정사항으로부터 동작 수정사항을 발라내느라 진을 뺄 필요가 없습니다. 이미 내가 2번에서 구분해두었으니까요!
이후 내용은 Part 2로 이어집니다.
