🛫 서론

우연히 한빛 미디어 컨퍼런스에 초대받았다. 주제는 코드 리뷰(우리 팀의 코드 품질 향상을 위한 코드 리뷰)로, 코드 리뷰를 받는 사람과 하는 사람 입장을 나누어 설명을 들을 수 있었다. 이 중 코드 리뷰를 받는 사람의 입장에서 필요한 자세가 인상깊어 자세히 정리해보려고 한다.

🔙 시작하기 전에...

컨퍼런스를 들으면서 몇 가지 새로운 표현들을 알게 되어 정리를 해보았다. 단순히 용어만 정리하기 보다는 어떤 맥락에서 나왔는지를 함께 정리해보았다.

1. LGTM 남발은 코드 리뷰 도입을 힘들게 한다.
   LGTM이란 Looks Good To Me의 약자로, 오픈 소스에서 별다른 코멘트 없이 approve를 줄 때 사용하는 표현이라고 한다. (아마도 아무 말 없이 approve를 주면 민망하다고 느껴서 생긴 표현이 아닐까 싶다.) 불필요한 코멘트를 남길 필요까지는 없지만, 다른 리뷰어가 코드 리뷰를 진행하리라는 안일함으로 LGTM을 남발하면 코드 리뷰 도입을 무의미하게 만듦으로 지양해야 한다.

2. on call과 MTTR : 코드 리뷰가 필요한 이유
   
   on call의 사전적인 의미는 '긴급 대기의'이다. 즉, 배포나 운영중 문제가 발생하였을 때 당직 개발자에게 연락하면 해당 개발자가 마치 '긴급 대기' 중인 것처럼 해당 문제를 해결해야 함을 의미한다. 문제는 조직의 규모가 클 수록 on call된 개발자가 해당 코드를 작성한 개발자가 아닐 확률이 높다.
   조직 내에 코드 리뷰를 도입하면, 개발자들이 코드 리뷰에 참여함으로써 상호 코드의 이해력을 높일 수 있고, MTTR(Mean Time To Recovery) 즉, 문제 발생으로부터 해결까지 걸리는 시간을 줄일 수 있다는 이점이 있다.

3. Bus factor : 또 다른 코드 리뷰의 필요성
   Bus factor는 '프로젝트를 진행하는 팀원 중 몇 명이 갑작스럽게 빠지게 되었을 때 프로젝트가 중단 내지는 그에 준하는 심각한 상황에 놓이는지'를 나타내는 지표이다. 팀 구성원 간에 공유되지 않는 정보 및 기능으로 인해 발생하는 위험을 나타내기 위해 사용되며, '팀원이 버스에 치였다면...'을 가정하는 표현을 사용하면서 Bus factor라는 이름이 붙었다. (누가 이런 무시무시한 상상을...?🤔🤔🤔)
   팀 내에서 코드 리뷰를 진행하면 모두가 코드 내용을 이해하고 공유하고 있는 context sharing 덕분에 Bus factor가 최소화된다.
   

😀😀😀 그래서 코드 리뷰를 '잘' 받으려면...?

용어를 정리하면서 자연스레 코드 리뷰가 왜 필요한지를 설명해 보았으니, 지금부터는 코드 리뷰를 '잘' 받기 위해 어떤 준비를 해야 하는지 정리해보겠다.

코드 리뷰를 받기 위해 중요한 것은 리뷰어가 어떤 관점에서 어떤 리뷰를 할 지 빠르게 알 수 있도록 준비하는 것이다. 즉, 리뷰어가 가지는 코드 리뷰의 부담을 줄이고 효율을 높여야 한다.

🫰 Git의 올바른 사용

아마 대부분의 팀에서 Git을 사용하여 코드를 관리할 것이다. 이 때 Git의 관리 방법을 팀 내에서 합의하고 이에 맞추어 관리하는 것이 중요하다.

🚓 브랜치 관리 전략 준수

브랜치 관리 전략에는 git flow, github flow, gitlab flow 등이 있다.

현재 우리 팀에서는 git flow를 변형하여 사용하고 있다. 기본적으로 git flow는 master, develop, feature, release, hotfix 5개의 브랜치를 나누어 관리하고, 각 역할은 다음과 같다.

1. master : 현재 제품의 최신 상태를 나타낸다. master 대신 main을 사용하기도 한다.
2. develop : 다음에 출시할 기능을 개발중인 브랜치이다.
3. feature : develop 브랜치에 새로운 기능을 추가할 때 사용한다.
4. release : 출시 버전을 관리하며 최종적인 리뷰 진행 후 master로 merge한다.
5. hotfix : 지금 당장 수정해야 하는 치명적인 오류를 해결하기 위해 master 브랜치에서 브랜치를 새로 따 해결 후 직접적으로 merge한다.

이러한 브랜치 관리 전략을 통해 리뷰어는 단편적으로 개발자가 어떤 코드를 작성한 것인지, 급하게 리뷰해야할 코드인지를 알 수 있다.

예를 들어, feature 브랜치에서 develop으로 합치는 pr이 왔다면, 해당 pr만으로 개발자가 새로운 기능을 개발하여 추가하였구나를 알 수 있다. 따라서, 리뷰어는 새로운 기능 추가에 초점을 맞추어 리뷰를 진행하면 된다.

만약 hotfix에서 master 브랜치로 합치는 pr을 받았다면, 리뷰어는 해당 pr이 시급한 사안이라고 판단할 수 있다. 추가로 hotfix 브랜치를 따기 전 어떤 오류가 있었는지, 수정한 후 새로 생기는 오류는 없을 지 등에 집중하여 리뷰를 남길 수 있다.

🤙 commit conventions

Git에서 저장소에 업로드하기 위해선 commit을 작성한다. 해당 commit에는 commit message를 작성하여 함께 작업하는 팀원이, 혹은 미래에 다시 이 코드를 보게 될 본인 스스로가 이해할 수 있도록 한다. 팀 내부에서 합의된 commit conventions은 동료로 하여금 어떤 작업을 했다를 알 수 있도록 돕는다.

예를 들어, pr이 단 3개의 커밋으로 구성되어 있고, 해당 커밋 메시지들이 feat - perf - perf 로 구성되어 있다면, 직관적으로 기능을 하나 추가한 후 해당 성능이 만족스럽지 않아 성능 개선 작업을 진행했음을 판단할 수 있다. 따라서 리뷰어는 커밋 메시지만으로도 이번 코드 리뷰는 성능 개선에 집중하여 진행해야겠다는 깨달음을 얻을 수 있다.

conventional commits를 통해 대표적인 commit conventions을 확인해볼 수 있다.

이처럼 팀 내에서 합의된 브랜치 관리 전략과 commit convention을 준수함으로써 리뷰어가 어디에 집중하여 코드 리뷰를 진행할지 직관적으로 파악할 수 있도록 할 수 있다.

🙆 리뷰받을 준비가 된 PR

코드 리뷰를 잘 받는 개발자가 되기 위해선 코드리뷰를 요청하기 전, 최소한의 조건을 만족하는지 살펴야 한다.

👌 CI 파이프라인 통과 여부

CI(Continuous Integration : 지속적 통합) 파이프라인은 개인이 작성한 코드가 팀의 코드로 인정받기 위한 준비가 되었는지를 확인시켜준다. 이를 돕는 두 개의 툴을 소개받았다.

1. formatter
   formatter는 팀에서 합의한 형태를 갖추고 있는지를 검사해주는 도구이다.

2. type checker
   type checker는 말 그대로 type을 check해주는 도구이다. 파이썬 같은 언어는 type이 틀려도 동작하므로 개발자의 혼돈을 야기할 수 있다. type checker를 통해서 이를 방지할 수 있다.

👌 Unit Test

Unit Test를 모두 통과하는 것 역시 코드가 merge되기 전 필수로 확인해야 하는 사항이다. 이때 주의할 점은 Unit Test를 통해 나의 코드가 타인의 코드를 해치지 않았는지를 확인해야한다는 것이다.

따라서 Unit test가 스스로의 컴퓨터뿐만 아니라 타인의 컴퓨터에서도 동일하게 성공함을 보장하면서도 기존 코드의 테스트가 실패하지 않는지도 확인해야 한다. 이를 간편하게 처리하기 위해 CI 파이프라인에서 자동화시켜 확인하도록 설정할 수 있다.

👌 PR이 리뷰 가능한 크기인가?

현재 프로젝트에서 프론트엔드가 나 하나뿐이라 나도 종종 저지르는 실수인데, PR이 리뷰 가능한 크기인지도 성공적인 코드 리뷰에 중요한 요소이다.

만약, 어쩌다보니 커밋이나 변경 사항이 너무 많이 쌓였다면 내 브랜치에서 특정 기능 단위로 브랜치를 뜯어서 PR을 날리는 방법이 존재한다.

👌 코드 리뷰 설정

1. 적절한 리뷰어 설정
   깃허브 PR에서 리뷰어를 설정할 수 있는 기능을 이용할 수 있다.

PR 우측에 위치한 Reviewers를 통해 PR에 적절한 리뷰어를 설정할 수 있다. 그동안 개인 리뷰어만 지정했었는데, 팀 리뷰어를 지정할 수 있다는 사실을 새로이 알게 되었다. 만약 팀 리뷰어를 지정한다면 팀 자체를 태그할 수도, 팀원 중 랜덤하게 태그할 수도, 모두에게 공평할 수 있도록 round robin 방식으로 태그할 수도 있다.

2. branch protection rule
   branch protection rule은 merge하기 위한 조건을 자동으로 판단해주는 설정이다. 해당 설정의 대표적인 예시로는 바로 master 브랜치로 merge할 수 없도록 제한을 거는 설정이 있고, 현재 우리 팀은 1명 이상의 approve를 받아야지만 merge할 수 있는 설정을 추가해두었다.

3. code owner
   code owner는 마치 코드의 주인처럼 특정 코드를 수정하게 되면 꼭 리뷰해야하는 사람을 지정해둘 수 있다.
   현재 진행중인 프로젝트를 예시로 들자면, 세 명의 개발자가 각각 크롤링, 크롬 익스텐션, 프론트엔드를 거의 전담하여 개발하였다. 그 중 가끔 다른 개발자가 해당 영역에 오류를 수정하거나 코드를 작성하는 경우가 있는데, 본인이 전담했던 파트가 아니다보니 코드를 수정할 때 고민이 많이 되었었다. 이번 발표를 들으면서 크롤링, 크롬 익스텐션, 프론트엔드 레파지토리에 code owner를 적용해보면 좋겠다는 생각이 들었다.

👌 self-review

그동안은 데일리 스크럼에서 '어떠한 문제점이 있어서 현재 어려움을 겪고 있다'거나 '이러한 현상이 발생하고 있는데 이 기술을 사용하면 해결할 수 있을 것 같다'등의 고민을 나누어 왔었다. 다만, 해당 방식은 짧게 끝내야 하는 데일리 스크럼의 특성상 아주 큰 문제일 경우에만 이야기를 나눌 수 있었다.

self-review란 코드를 작성하면서 했던 고민이나, 다른 대안, 문제점 등을 작성해두는 것이다. 작성했던 코드에 직접적으로 언급을 함으로써 코드 리뷰어도 눈에 보이는 예시를 통해 더 이해하기가 쉬울 것이고, 나 역시도 보다 사소한 문제까지도 나누며 문서화할 수 있어 좋은 방식인 것 같다.

🍿 후기

불과 이틀 전에도 컨퍼런스에 다녀와서 집중력이 많이 떨어지리라 예상하였는데 오히려 당장 내일부터라도 적용하고 싶은 다양한 방법들을 얻을 수 있었다.

또, 시작할 때 해당 컨퍼런스를 1~3년차 개발자를 대상으로 준비하셨다고 말씀하셔서 혹시나 이해하지 못할까봐 걱정이 많았는데, 용어마다 최대한 쉽게 풀어서 설명하시는 강연자님 덕분에 흥미롭게 들을 수 있었다. (이게 바로 Tech Lead의 품격...?)

추가로, 바로 이전 컨퍼런스 주제였던 TDD가 가장 듣고 싶었는데, 곧 한빛n 사이트에서 추후 다시 들을 수 있다고 한다! (두구두구두구...!)