이 글은 코드 리뷰의 목적, 기법을 설명하기 보단, 리뷰어가 읽기 좋은 PR, 작성자가 읽기 좋은 리뷰를 만드는 것에 대한 개인적인 생각입니다.
일을 하다 보면 일관성 없는 PR, 가독성 떨어지는 코드 리뷰를 마주할 때가 간혹 있다. 아마 개인의 성향, 스타일, 컨벤션, GitHub 활용 미숙 등으로 생겨났을 것이다. 이러한 부분을 조금만 신경쓰면 (읽기) 좋은 PR, 코드 리뷰가 되지 않을까?
PR ≠ 코드 리뷰
Git & GitHub(혹은 PR 기능이 있는 호스팅 플랫폼) 기반에서 PR을 활용한 브랜치 전략을 사용한다고 가정해보자. 온라인 코드 리뷰를 받기 전에는 반드시 필요한 사전 작업이 있을 것이다.
브랜치 생성 → 코드 추가/변경 → Commit → Pull Request → 코드 리뷰위와 같은 일련의 과정을 거쳐야 팀원들은 해당 PR을 코드 리뷰 할 수 있다. PR과 코드 리뷰는 다른 행위이고, 이 글에서는 이 둘을 분리하여 생각해봤다.
좋은 Pull Request란?
‘전달력 있는 PR’부터 만들자.
좋은 PR에 좋은 코드 리뷰가 달리기 마련이다. 어떤 작업인지 나열하는 것보다 핵심을 전달하는 것이 중요하다고 본다. 구구절절하거나 온갖 commit이 섞여 있는 PR은 코드 리뷰하기 꺼려진다.
# 제목을 잘 작성하자
담당자가 가장 먼저 작성하고, 리뷰어가 가장 먼저 보는 것이 ‘제목’이다. 당연히 제목을 제목답게 잘 작성했을 테지만, 몇 가지 제목에 대해 고민한 점을 남긴다.
PR은 커밋이 아니다. 팀 내 커밋 컨벤션을 PR에 그대로 적용하지 말자. 한 개의 커밋만 있는 PR일 경우 커밋 메세지가 자동으로 PR 제목으로 설정되는데 이를 그대로 사용하여 PR 컨벤션으로 정하지는 말자. 두 개념은 역할도 다르고 의미하는(포함하는) 범위도 다르다.
PR 제목은 리뷰어/팀원들을 위해 작성하지만, 커밋의 제목(subject)은 아니다.
키워드를 활용하자. 하지만 컨벤션으로 강제할 필요는 없다. PR은 한번 머지되고 난 이후에 다시 찾아볼 일이 거의 없다. 머지되기 직전까지 본인 혹은 리뷰어의 이해도를 높이기 위해 ‘잘' 작성하면 된다. 추상화된 작업으로 분리할 수 있다면 라벨을 활용하는 방안도 있다.
이슈 번호 접두어를 사용하지 말자. 이슈 번호를 PR 본문에 적으면 GitHub 기능들을 활용할 수 있다. 하지만 제목은 다르다. GitHub이 제목에 autolink를 만들어주는 것도 아니고, 제목은 대부분 PR로 이동하는 링크여서 이슈 번호만 드래그&복사하기도 불편하다. PR 본문에 들어오면 이미 담당자가 autolink로 잘 연결된 이슈 번호를 남겨놨을 것이다(혹은 PR-이슈 연결). 다시 말해, GitHub 혹은 리뷰어, Tool에서 얻을 수 있는 이점이 없다. 리뷰어가 PR 내용을 파악하기 위해 불필요한 접두어를 읽어야 하는 비용만 있을 뿐이다.
# 본문을 잘 작성하자
제목 못지않게 본문을 잘 작성하는 것도 중요하다. 개인마다 작성하는 스타일이 있겠지만 어느 정도 작성 방법(컨벤션)을 정하면 모두가 같은 순서으로 읽으니 빠르게 이해하고 코드 리뷰하는 데 도움이 될 것이다.
코드의 맥락을 이해할 수 있는 설명을 작성하자. 해당 코드가 어떤 목적을 가지고 작성 되었으며, 왜 필요한지 맥락을 짧게나마 작성해두면 리뷰어가 해당 코드를 빠르게 이해할 수 있고 핵심에 대한 리뷰를 해줄 가능성이 높아진다.
PR 템플릿을 사용하자. GitHub에서는 PR 생성 시 본문에 대한 템플릿을 만들어 일관성을 갖도록 도와준다. <!-- —-> 으로 주석도 남길 수도 있으니 PR 생성자에게 가이드 해줄 수도 있다.
<!-- Linked issues -->
org/repo### Summary## Test Plan
<!-- 테스트 방법 설명 혹은 TC를 추가했는지-->## Related services
<!-- 타 서비스와 의존성이 있을 경우 설명 -->## References
<!-- 관련 자료가 있을 경우 첨부 및 링크 추가 -->
✅ NOTE: 이슈에도 템플릿을 활용할 수 있다(ref). 여러 템플릿이 나올 수 있는데, 작업의 시작을 나타내는 이슈일 경우 아래 ‘Tech Spec’을 활용해도 괜찮을 것 같다.
- How to write AWESOME Tech Specs — Lyft Engineering
- 뱅크샐러드의 특별한 스펙, ‘테크 스펙’ — 뱅크샐러드 블로그
🤔 여기서 잠깐! PR 템플릿과 이슈 템플릿을 비교해보면 PR 템플릿에는 제목과 관련된 설정이 없다. PR 생성자가 직접 ‘해당 작업을 내용을 작성하라’고 말하는 게 아닐까?
오토링크(Autolinked reference)를 활용하자. 본인 혹은 리뷰어를 위해 이슈나 이전에 언급된 코맨트, 커밋 등 관련된 정보들을 연결해주는 것이 좋다. GitHub은 URL과 특정 형식으로 작성된 GitHub 내부 레퍼런스를 자동으로 링크로 만들어준다. 또한, GitHub 이외의 서비스(JIRA issue 등)를 위해 Custom autolink도 지원한다.
관련 있는 이슈와 연결하자. 위에 작성한 템플릿의 첫째 줄을 보면 org/repo#<id> 형식으로, 관련 있는 이슈의 링크(오토링크)를 남기도록 가이드했다. 이렇게 이슈 번호를 남기면 해당 이슈에 PR이 연결된다. 직접 연결하는 방법도 생겼지만 GitHub Enterprise 2.21 이후에만 적용되어 있음으로 본문에 오토링크를 작성하는 것을 사용하자. 본문에 작성하면 IntelliJ에서도 볼 수 있으니 유용하다.
추가로, 이슈 번호를 남길 때 특정 키워드를 활용하면 PR이 완료될 때 이슈를 함께 닫을 수 있다(Close issue). 개념적으로 PR은 이슈를 처리한 작업이니 관련 있는 이슈를 닫는게 맞고, 이러한 작업의 편의성을 위해 GitHub이 키워드를 제공한 것으로 보이니 활용하자.
마크다운 문법을 활용하자. GitHub에서는 마크다운 문법과 관련하여 다양한 기능을 제공한다. 리뷰어를 배려하는 마음으로 몇 가지 기능을 활용해보자.
1) 인라인 코드 블럭
본문 내 코드는 백틱(backtick, `)을 사용하자(e.g., true, TestClass). 그리고 코드가 아닌 것은 bold 나 italic 을 활용하자(e.g., “핵심이다” → “핵심이다”). 가끔 코드가 아닌데도 불구하고 백틱으로 감싼 것이 있다. 기본적인 의미에 혼란을 주어서는 안된다고 생각한다. 다른 문법(스타일)으로 효과적으로 표현이 안된다고 할 경우에만 사용하자.
2) 코드 블럭
코드 블럭을 만드는 방법은 마크다운을 조금만 사용해봐도 알 수 있을 것이다. 왠만하면 코드 블럭을 만들 때 문법 강조(syntax highlighting) 기능도 활용해보자. 여러 언어들도 지원하고 diff 기능도 지원한다.
3) 체크 박스(≒ Take lists)
GitHub에서는 체크 박스 형태로 할 일 목록(Task list) 기능을 지원한다. PR 목록에서도 작업의 진척 상황을 보여주므로 활용할 만한 가치가 충분하다. 단순한 스타일 문법 수준으로 보지 말고 기능 관점으로 활용하자.
4) 취소선(Strikethrough)
Bold와 Italic을 주로 많이 사용할 것이다. Strikethrough도 있으니 적절한 곳에 활용하길 바란다.
5) 본문 접기(collapsible content)
내용이 많아서 접어두고 싶은 본문이 있을 수 있다. 아래와 같은 마크업으로 감쌀 경우 해당 내용을 접어둘 수 있다.
<details>
<summary>Details</summary>- item1
- item2</details>
리뷰어를 배려하자. 본문을 작성하면서 리뷰어들을 조금만 더 배려해보자. 예를 들어, 들여쓰기 수정 커밋 건이 있을 경우에 너무 많은 변경 건이 나올 수 있다. 리뷰어가 직접 URL에 ?w=1추가 혹은 UI를 활용할 수도 있겠지만 아래처럼 링크를 링크를 남겨주는 것도 괜찮을 것 같다.
# 작업을 적당히 분리하자 ⭐️
온갖 잡다한 커밋을 모아모아 하나의 PR을 만들고 좋은 코드 리뷰를 기대하지 말자. 거대한 PR일 수록 리뷰어가 많은 시간을 할애해야 하고, 리뷰하는 도중에 중요한 부분을 놓칠수도 있다. 그리고 리뷰를 자꾸 미루게 된다. 리뷰어의 입장도 생각해주자.
거대한 PR에서 많은 코드 리뷰를 받을 경우 수정해야 할 점들이 많아지고, 또 다시 리뷰 받고, 또 수정하고… 결국 머지 하는 데 까지 긴 시간이 걸린다. 점점 핵심과 벗어난 작업을 하거나 충돌을 걱정하는 일들이 생긴다. (구글의 경우 코드 리뷰의 90%는 10개 미만의 파일로 구성된다고 한다.)
PR을 나누기 어렵다면 커밋을 잘 나눠보자. 사람마다 커밋과 PR을 다른 의미로 본다. 누군가는 PR을 롤백하기 위한 단위로 사용하고, 해당 PR에 1개의 커밋만 있는 경우도 있다.
Git에서 커밋(or revision)은 특정 시점의 스냅샷, 개별 파일의 변경점이다. 누가 언제 변경했는지와 변경사항을 기록하기 위해 고유한 ID(hash)를 사용한다. PR은 변경 사항(1개 이상의 커밋)을 origin에 제안하고 논의할 수 있는 기능이다. 호스팅 업체마다 목적은 같지만 부가적인 기능이 다르다. (GitHub의 경우에는 PR을 revert하는 기능이 제공된다.) 그러므로, 커밋을 기반으로 가능한 한 작게, 롤백이 가능한(or 체리피킹이 가능한) 단위로 나누는 것이 좋다.
# 리뷰어(Reviewer)를 잘 설정하자
심폐소생술 교육 받을 때 119에 신고할 사람을 꼭 집어서 요청 하라는 말이 있다. 코드 리뷰에서도 동일하다고 본다. 물론 팀원들이 자발적으로 리뷰 해준다면 더할 나위 없지만 여건상 계속 미뤄지기 마련이다. 이제 코드 리뷰를 기다리는 담당자는 누군가 리뷰(approve) 해줄 때까지 한 없이 기다릴 뿐이다.
팀 규모가 작으면 상관없겠지만, 습관적으로 모든 팀원을 리뷰어에 넣지 말자. 간혹가다 설정을 통해 팀을 혹은 모든 팀원을 넣는 경우가 있는데 이건 안넣는 것과 다를 바가 없다.
GitHub은 PR을 생성할 때 Reviewers에 해당 코드 관여자를 제안(suggestions)해준다. 또한 CODEOWNERS 에 모듈(directory)별로 설정해두면 리뷰어를 자동 지정해준다. GitHub 기능을 활용하면 담당자를 좀 더 명확하게 혹은 쉽게 알 수 있으니 Reviewer를 의미있게 지정하자.
업무 메신저에 봇 기능이 있다면 GitHub 웹훅을 활용해보자. 메신저로 리뷰어 할당을 알려준다면 알람 관련 확장프로그램이나 직접 확인할 필요가 없으므로 생산성에 도움이 될 것이다.
# 우선 순위 혹은 중요도를 나타내자
리뷰어들은 어떤 PR을 먼저 봐야할지, 얼마나 중요하게 확인해야할지 모른다. 물론 함께 진행하는 프로젝트라면 알겠지만, 대부분 잘 작성된 본문 혹은 코드를 봐야지 알 수 있다. 리뷰어가 다른 업무로 바쁠 경우 아예 챙기지 못할 수도 있다. 프로덕트에 반영해야하는 기간은 다가오고 담당자는 마음만 급해질 것이다.
팀 내에서 마일스톤(Milestone)을 사용한다면 PR에도 적용해보자. 코드 리뷰 마감일을 정의해두면 리뷰어들은 어떤 PR부터 리뷰해야 할 지 파악하기 쉬워진다. 마일스톤 순으로 정렬하여 볼 수도 있다.
마일스톤 말고 코드 리뷰 우선순위를 나타내는 방법은 많다. PR 템플릿에 체크박스를 활용할 수도 있고, 라벨을 활용할 수도 있다. 라벨을 사용할 경우 hook을 받을 수 있으므로 확장된 기능을 제공할 수 있다. 예를 들어, 우선순위가 높은 PR이 생성될 경우 봇(bot)을 통해 지정된 리뷰어에게 알람을 보내 담당자가 직접 찾아갈 일을 없앨 수 있다.
개인적으로 체크박스(Task list)보다 라벨이 적합한 것 같다. 체크박스는 본문에 바로 표시할 수 있지만, 본래의 체크박스 기능을 활용할 수 없는 단점이 생긴다. PR 리스트에서 항상 2개의 하위 작업이 마무리가 안된 상태로 보이거나 본문에서 중요한 내용보다 큰 영역을 차지한다.
# Draft Pull Request를 활용하자
한동안 여러 오픈소스들이 WIP (work-in-progress)라는 라벨 혹은 접두어를 활용하는 경우가 많았다. 이러한 PR은 아직 개발 진행 중이지만 CI/CD 프로세스를 태우거나, 일부 코드 리뷰를 받거나, 논의가 필요하거나, 브랜치를 자주 지우는 경우 아직 머지 안 된 브랜치를 나타내는 등 다양하게 활용했다. PR 제목에 [WIP] 접두어가 있으면 머지 버튼을 막는 크롬 확장 앱도 있었다.
2019년 쯤, GitHub PR에 draft pull request가 추가되었다. 이제 UI를 통해 코드 리뷰를 기다리는 PR과 작업 진행중인 PR을 구분하기 쉬워졌으니 활용해보자.
# 브랜치명 컨벤션을 정하자
어느정도 브랜치명을 정해놓으면 요긴하게 쓰인다. 일관성을 가진다고 나쁠건 없다. gitflow 브랜치 전략을 사용하고 있다면 feature/, hotfix/, release/ 형식으로 사용하고 있을 것이다. 여기서 약간만 추가해보자.
feature/jun/update-user-repository
feature/delete-user-service브랜치명은 kebab-case를 사용한다. GitHub에서 브랜치명은 URL로도 쓰인다. 사실 구분자로 _(hypen), -(dash) 어느 것을 쓰던 상관 없지만, URL은 보통 소문자로 사용하니 URL 형식으로 많이 쓰이는 kekab-case를 제안한다.
브랜치명에 author는 Slash(/)로 구분하자. 협업을 하다보면 브랜치명에 author를 작성하는 경우가 있다. 각각 다양한 구분자를 사용하지만 이를 Slash(/)로 구분하는건 어떨까? 이렇게 사용할 경우 IntelliJ에서 브랜치 영역에 디렉토리로 관리해준다.
# 커밋의 제목(Subject)을 잘 작성하자
PR은 커밋을 모아놓은 작업 단위다. 좋은 커밋들이 모아져야 좋은 PR이 만들어지고 좋은 코드 리뷰가 일어날 것이다. (대부분 diff 부터 볼테지만) 리뷰어는 본인의 스타일에 맞게 커밋 혹은 파일 별로도 볼 수 있을 것이다. 작업의 단위를 잘 나누고, 해당 작업에 대한 제목(Subject)을 잘 작성해두자. 그럼 PR의 커밋 목록을 보는 것 만으로도 어떤 작업인지 알 수 있으며, 어떤 작업(커밋)에 연관된 변경인지도 파악하기 쉽다.
사실 커밋은 리뷰어를 위한게 아니라 형상관리를 위한 것이므로 스스로 목적에 맞게 잘 나누고, 의미있는 subject, body를 작성하자.
# 히스토리를 잘 관리하자
히스토리를 보면 프로젝트가 어떻게 진행되었는지 볼 수 있다. PR 내에서도 동일하다. PR의 단위를 작게 가져가는 것이 좋겠지만, 작업의 단위가 크거나 다소 긴 시간동안 머지되지 않을 수도 있다. 담당자는 main 브랜치의 변경을 반영하거나 충돌(conflict)을 해결하기 위해 머지하게 되는데, 이 때 발생하는 머지 커밋(merge commit)이 히스토리를 보는데 방해 요소가 될 수 있다.
히스토리를 가꾸기 위해 Git을 좀 더 활용해보자. 혼자 작업하는 브랜치일 경우 작업 일시를 남길 필요 없다면 main 브랜치를 rebase로 머지해도 된다. 이전 커밋에 추가 변경 사항이 발생했을 경우 commmit --amend로 반영할 수 있다. 잘못 커밋 했을 경우 revert하지 않고 reset을 활용할 수 도 있을 것이다. 백업을 위해 여러 커밋으로 나눴다가 작업이 어느정도 정리되면 rebase -i 로 커밋을 합칠 수도 있다. (참고: Git 브랜치 — Rebase 하기)
PR을 머지할 때에도 다양한 방법이 있으니 활용해보자.
참고 링크:
- Rebase vs. Merge — git documentation
# CI/CD를 활용하자
시간이 여유롭다면 CI/CD를 활용하여 생산성을 높혀보자. 누군가 한명이 구축해둠으로써 팀내 전체 생산성이 증가한다. 정적 분석이나 린트 결과를 PR의 마지막 커밋에 적용해두는 것이다. 이를 통해 리뷰어는 자잘한 컨벤션보다 핵심 로직에 집중하여 코드 리뷰를 해줄 것이다.
# 여기까지
의미있는 PR을 만드는데에도 적지 않는 고민과 비용이 든다. 문서란 다 그런게 아닐까… 나중에 볼 사람들을 위해 잠깐만 시간을 할애하자. 이런 기능들은 한 두 번 쓰다 보면 금방 익숙해질 것이다. ‘나중에 볼 사람들’이라고 했지만 본인도 포함될 경우가 많다.
좋은 코드 리뷰란?
코드 리뷰도 가독성을 높이자.
코드 리뷰에 대해 검색해보면 프로세스, 중요성, 팁(tip) 등 다양한 관점에서 잘 작성된 글들이 많지만, 이 섹션 또한 PR과 마찬가지로 가독성 측면에서 고민해보았다.
# 마크다운을 활용하자
우선 위(‘# 본문을 잘 작성하자’)에 작성한 마크다운 내용을 한번 더 참고 바란다. 여기서는 다른 기능을 소개하고자 한다. suggestion 코드 블럭을 추가하면 변경 사항 제안 기능을 사용할 수 있다. 이를 통해 담당자는 버튼 클릭 한번으로 제안된 변경점을 코드에 반영할 수도 있다.
# 관련 링크를 연결해주자
코드 리뷰시 보통 자신의 지식에 기반하여 리뷰하게 된다. 내가 어떤 책, 문구, 지식, 링크를 통해 이러이러한 생각이 들었는지 참고 링크(혹은 인용문)를 같이 전달해주는 건 어떨까? 좀 더 설득력있지 않을까? 사고의 확장에도 도움이 되지 않을까?
관련 코드를 첨부할 수도 있다. 이 때 GitHub의 permanent link를 통해 code snippet을 코멘트가 추가할 수 있다.
# 리뷰 반영 링크를 남겨주자
코드 리뷰를 PR에 반영하고 난 뒤에 해당 코멘트에 커밋 링크를 남겨주자. 리뷰어가 다시 리뷰할 때 도움이 될 것이다. 커밋 URL을 첨부하면 GitHub에서 Short link로 자동으로 만들어준다(Commit SHAs). 여러 커밋으로 나눴으면 Comparing commit 링크를 남겨주는 방법도 있다(e.g., a52312…dd11231).
미래를 위해 코드 리뷰를 반영한 커밋에도 해당 리뷰 링크를 추가해보자. 커밋 메세지에 ‘리뷰 반영'만 남기는 것보다 도움이 된다.
# GitHub 기능을 활용하자
2020년 2월 쯤에 Multi-line 코멘트 기능이 추가되었다. 기존에 한 줄씩 리뷰해야 했는데, 이젠 여러 줄에 대해서도 리뷰를 남길 수 있으니 활용하자.
# 약어를 활용해보자
많은 오픈 소스 영역에서 이뤄지는 코드리뷰를 보면 약어를 사용한다. ‘WIP(work in progress)’, ‘LGTM(looks good to me)’ 등이다. 우리는 코드 리뷰 승인(approve)할 때 ‘고생하셨습니다’, ‘수고하셨습니다’ 등으로 표현하는데, ‘LGTM’으로 보다 간편하게 의미를 전달할 수 있어 용이하다.
‘Nit’는 약어보단 태그와 같이 사용할 수 있는데, 이는 중요하진 않은(반드시 수정해야만 하는 것이 아닌) 리뷰를 뜻한다. 코드리뷰 할 땐 다양한 의견을 건내고 논의해야 함께 성장한다. 반드시 고쳐야 하는 리뷰, 그렇게 받아들이게 되는 어휘가 계속된다면 코드 리뷰는 수직적인 절차가 될 것이다.
- LGTM — “Looks Good To Me”
- IMO — “In My Opinion”
- SSIA — “Subject Says It All”
- WIP — “Work In Progress”
- TL;DR. — “Too Long. Didn’t Read.”
- Nit — “Nit-pic” (ref)
참고 링크:
- What do cryptic Github comments mean? — freeCodeCamp
- Github: 은근히 많이 쓰는 깃헙약어 — 히데쿠마 개발블로그
- 구글 코드 리뷰 가이드
# Emoji를 활용해보자
2016년쯤 GitHub의 PR, 이슈, 코멘트에 이모지(Emoji)로 표현하는 기능이 추가되었다. 코드 리뷰에 대한 빠른 피드백을 위해 이모지를 활용해보자. 대부분 의미가 명확하며, 👀 이모지는 지켜보거나 조사하는 중에 사용하면 된다.
참고 링크:
- Git Commit message Emoji — parmenft’s gist
- Code Review Emoji Guide — erikthedeveloper
# 코드리뷰할 PR 빠르게 확인하기
GitHub Enterprise 2.21에 @me 검색 쿼리가 추가되었다. 기존에 제공하던 “Custom tabs”를 활용하면 코드 리뷰가 필요한 PR 목록을 볼 수 있는 링크를 만들 수 있다. 이를 통해 팀원들에게 리뷰가 필요한 리스트로 바로 이동할 수 있게 도와줄 수 있다.
GitHub Enterprise 2.22에서 UI가 변경되면서 “Custom tabs” 추가시 드롭다운 버튼이 아닌 네비게이션에 바로 노출된다.
# 여기까지
코드 리뷰를 조금 더 잘 작성하는 방법에 대해 적어보았다. 코드 리뷰도 PR과 마찬가지로 언젠가는 본인을 포함한 모두가 볼 수 있고, 이미 논의된 내용을 다른 PR에서 링크로 참조할 수도 있다. 그러니 이왕이면 작성할 때 조금만 더 신경써보자.
마치며…
이렇게 나의 PR을, 커밋을 잘 작성하려고 보고 수정하다보면 자신의 PR을 스스로 코드리뷰 하는 것과 비슷하다고 생각한다.
이 모든 것은 현시점에서 지극히 주관적으로 생각/고민한 ‘좋은 PR과 코드 리뷰’에 대해 말한다. 다시 말해, 정답은 아닐 뿐더러 얼마든지 내 주관도 바뀔 수 있다는 말이다.
다른 의견이 있다면 언제든지 편하게 댓글을 남겨주었으면 한다.
참고 링크
- Five Code Review Antipatterns — Oracle
- 효과적인 코드 리뷰를 위해서 — LINE Engineering
- 우리는 코드 리뷰를 잘하고 있을까요? — Style Share
- 코드리뷰의 진짜 목적은 따로있다. — 로지스팟
- CodeReview에대해 — 백명석
- 기술 업계의 독성 말투 문제, 고칩시다! — edykim/ko
