두 달 뒤면 카카오스타일 입사 2주년이 됩니다. 그동안 iOS 개발자가 계속 늘었는데, 동료가 많아지는 만큼 리뷰해야 하는 PR도 점점 늘어났습니다. 하루 종일 코드 리뷰만 하는 날도 있었고, 종종 너무 큰 PR은 적당히 마무리했던 적도 있습니다 🥹

code-review

이번 글에는 더 나은 코드 리뷰 문화를 만들기 위해, 제 나름대로 노력하고 있는 방법을 정리해보려 합니다.

바쁘신 분들을 위한 3줄 요약 😇

📝 커밋 히스토리를 깔끔하게 유지하자.
🔀 브랜치를 적절히 사용해서 PR을 나누자.
⚡️ 내 코드를 리뷰하는 동료들의 시간은 소중하다.

커밋

커밋 메시지는 리뷰어에게 이정표와 같은 역할을 합니다. 잘 정리된 커밋은 코드 리뷰에 큰 도움이 되지만, 그렇지 않은 경우 리뷰를 어렵게 만드는 장애물이 됩니다.

저는 아래 3가지 원칙을 생각하며 커밋을 합니다.

커밋에도 순서가 있다

의식의 흐름대로 작업하고 마구잡이로 커밋하다 보면, 언제 어디서 어떤 작업을 했는지 찾기 어려울 때가 있습니다. 반대로, 자신만의 기준을 정해서 커밋을 하면, 어떤 작업이 어디에 있는지 손쉽게 찾을 수 있습니다.

예를 들어, 저는 하나의 화면을 새로 만들 때 ‘파일 추가 - View 구현 - ViewModel 구현 - 바인딩’과 같은 순서를 지키며 커밋을 올립니다. 그리고 앞에서 커밋한 코드를 뒤에서 다시 수정하는 것은 지양하고, 리베이스를 통해 커밋을 정리합니다.

커밋은 작을수록 좋다

지금까지 작은(작업) 단위로 커밋을 만드는 것이 대부분의 경우에 좋았습니다. 개인적으로는 하나의 커밋에는 하나의 작업 단위만 포함하는 것이 좋으며, 1~2줄의 메시지로 포괄할 수 있는 규모가 적절하다 생각합니다.

커밋 메시지에 의미를 부여하자

메시지에 의미를 포함하면 코드 리뷰에 큰 도움이 됩니다.

commit-1

위 스크린샷 처럼, 한 줄의 코드를 수정한 PR을 리뷰하는 입장에서 한번 생각해보겠습니다.

deselectItem 함수 호출 추가

첫 번째 메시지는 수정한 코드를 그대로 해석한 느낌입니다. 어떤 의도로 작업했는지 파악해야 하기 때문에, 코드 리뷰의 속도가 저하됩니다.

scroll 완료 후 결정되는 index로 select 상태를 갱신하기 위해, didSelectItamAt:에서는 deselect 처리

두 번째 메시지는 좀 더 많은 정보를 담고 있기 때문에, 작업자의 의도를 쉽게 파악할 수 있습니다.

브랜치

깔끔한 커밋과 적절한 규모의 Changes는 코드 리뷰 과정을 즐겁게 만들어줄 수 있습니다.

1000라인의 코드를 수정했는데, 하나의 PR에 모든 것을 담아내는 것과 2~3개의 PR로 나눠서 담아내는 것 중 어떤 것이 더 리뷰에 효율적일까요? 작업의 난이도에 따라 다르겠지만, 일반적으로 2~3개로 나눠져 있는 PR을 리뷰하는 것이 더 효율적입니다.

브랜치를 잘 나누는 것이 곧 PR을 나누는 것으로 이어집니다. 저는 아래 3가지 방법을 통해 브랜치를 나누고 있습니다.

단일 브랜치

pr-1

소규모의 작업이 이 케이스에 해당합니다. 수정한 코드가 너무 많을 경우, 아래 2가지 방법을 사용해 여러 개의 브랜치로 분리하게 됩니다.

수직적 브랜치

pr-3

이전 작업이 다음 작업에 영향을 미치는 경우에 사용하는 패턴이며, 단일 브랜치 다음으로 자주 사용합니다.

Base 브랜치 또는 직전 브랜치에서 개발을 충분히 하거나, 개발 전 작업 범위를 잘 나눠놓는 것이 좋습니다. 그렇지 않을 경우 앞쪽 브랜치의 수정 사항을 수시로 반영해줘야 하는데, 이 과정은 매우 번거롭고 커밋 히스토리를 오염시킬 가능성이 높습니다.

수평적 브랜치

pr-2

Base 브랜치를 기반으로, 여러 가지 다른 작업을 진행할 경우에 사용합니다. 하위 작업 간에 Dependency가 없는 경우에 사용하지만, 생각보다 자주 사용되지는 않습니다.

Base 브랜치가 수정되는 경우, 리베이스를 통해 수정사항을 쉽게 반영할 수 있습니다.

브랜치를 잘 나누는 비법 🍯

테크 스펙을 분석하고 작업 단위를 미리 계획해놓으면, 브랜치를 쉽게 나눌 수 있습니다. 처음에는 어렵게 느껴지지만 조금만 연습하면 금방 익숙해집니다.

그럼에도 브랜치 나누기가 어렵게 느껴진다면, 모든 기능을 하나의 브랜치에서 개발한 뒤 중간중간 브랜치를 삽입하는 방법을 사용해보세요. 물론 커밋을 순서대로 잘 작성했다는 조건이 필요합니다.

Pull Request 만들기

커밋을 정리하고 브랜치를 적절히 나누었다면 이제 PR을 만들 차례입니다. 저는 보통 PR에 4개의 항목을 작성합니다.

pull-request

  • Background : 이 PR에 어떤 작업이 포함됐는지에 대한 내용을 요약합니다. 관련 문서, 작업 티켓, 디자인 가이드 등의 문서도 링크합니다.
  • Changes : 주요 수정 사항을 기재합니다. 핵심 수정사항은 커밋 Hash와 함께 기재하는 편이며, 리뷰어가 이해하기 어려울 것 같은 부분에 추가 코멘트를 작성하기도 합니다.
  • Test Guide : 이 작업을 테스트하기 위한 방법을 기재합니다. UI 작업이 병행된 경우 전/후 스크린샷을 첨부하기도 합니다.
  • Important : 리뷰어에게 전달하거나 논의하고 싶은 내용을 기재합니다.

PR에는 핵심만 정리하는 것이 매우 중요합니다. 하나부터 열까지 모든 것을 이해시키려 하면 리뷰어가 피로감을 느낄 수 있습니다.

tmt

마무리

저도 처음에는 시행착오가 많았습니다. 리베이스를 하면서 작업내용이 모두 날아간 적도 있었고, 커밋을 분리하지 못해 다 한줄한줄 다시 커밋을 작성했던 적도 있습니다.

그럼에도 이 방법을 꾸준히 실천하려는 이유는, 내 코드를 리뷰하는 동료들의 시간은 소중하기 때문입니다. 처음엔 일정 때문에 어려울 수도 있지만, 익숙해지면 생각보다 많은 오류를 예방할 수 있고, 생산성도 더 좋아질 것입니다.

또 다른 좋은 방법이 있다면 공유해주세요, 읽어주셔서 감사합니다 😄