[Github] 좋은 PR의 기준이 뭘까?

멘토님께 PR 리뷰를 요청드렸는데 PR 메세지 개선이 필요하다는 피드백이 있었다. 회사에서도 사용하고 있던 PR 양식으로 (물론 혼자 진행하는 프로젝트라 내가 임의로 만든 템플릿이다) 이를 개선하기 위한 고민의 과정을 기록하기 위해 글을 쓴다.

목차

• PR이란 무엇인가?
• PR은 왜 하는 거지?
• 좋은 PR 메세지를 위한 개선
• 작은 PR로 만들기
• 결론

---

 

PR이란 무엇인가?

내가 작업한 부분을 코드베이스에 포함시키기 위해 보내는 요청으로 작업으로 생긴 변경사항(수정, 추가, 삭제)를 기존 코드에 적용하는 과정을 의미한다. 

 

PR은 왜 하는 거지?

PR의 목적은 크게 세 가지로 볼 수 있으며 각 항목을 기준으로 필수로 있어야 하는 내용들을 생각해봤다.

변경사항 기록

코드의 변경사항들을 기록하기 위해 PR을 작성하는 것이라고 생각한다. 물론 commit 단위의 메세지도 작성하지만 여러 commit을 모아 전체적인 변경 사항을 기록하고 히스토리를 남기기 위해 PR을 작성하는 것이라고 생각된다.

따라서 코드 관점에서 구체적인 변경 사항(예를 들면, 새로운 클래스 추가나 함수 내부 로직 수정과 같은)과 해당 작업을 진행한 이유가 있어야 읽는 사람도 작업을 이해할 수 있을 것 같았다.

또한 왜 그렇게 작업했는지? 구현 방법을 고민했다면 왜 이 방법을 택했는지? 어떤 트레이드오프를 고려해 결정했는지도 작성하면 코드를 이해하는데 더 도움이 될 수 있지 않을까 싶다.

마지막으로 변경 사항에 대한 테스트도 필요하다. 코드를 작성한 사람은 나 자신이기 때문에 주인의식을 가지고 코드에 대한 책임을 져야 하기에 스크린샷이나 테스트, 혹은 테스트를 진행하지 않은 경우 그에 대한 이유도 필요하다.

코드 병합

작업 사항을 베이스 브랜치에 병합(merge)할 경우 원본 코드에 영향을 줄 수도 있기에 사이드 이펙트가 존재하는지, 존재한다면 구체적으로 영향 범위가 어느정도인지 작성해야 한다.

코드 리뷰

코드 리뷰어가 있다면 작업물을 쉽게 이해할 수 있도록 도움이 필요하다. 아무 설명 없이 리뷰를 요청한다면 코드를 이해하기 어렵고 코드 리뷰 시간이 길어지기에 병목 현상이 발생할 수 있기 때문이다.

따라서 리뷰어는 사전 지식이 없는 상태에서 리뷰에 참여한다는 가정으로 리뷰 요청자는 충분한 문맥을 제공해야 한다고 생각된다. 

충분한 문맥을 제공하기 위해선 변경사항을 구체적으로 기록하는 것이 중요하다. 하지만 코드를 보는 것만으로 이해하기 어렵다면 코멘트를 통해 부가설명을 작성해야 하며 리뷰어가 중점적으로 봐줬으면 하는 부분들을 추가할 수도 있다. 

또한 코드 리뷰는 일방적인 소통이 아닌 리뷰 요청자와 리뷰어의 커뮤니케이션이기에 코드 리뷰어에게 질문을 함으로써 상호간의 소통을 할 수 있다.

마지막으로 코드에 대한 설명을 잘 작성해도 PR의 코드 라인 수가 너무 많으면 리뷰어의 집중도가 떨어질 수 밖에 없기 때문에 작은 PR을 위한 PR 라인 수에 대한 규칙이 필요하다.

 

좋은 PR 메세지를 위한 개선

PR 템플릿에 필요하다고 생각되는 내용을 바탕으로 아래와 같이 개선해봤다.

개선 전

기존 PR 템플릿

기존 PR title

개선 후

개선 후 PR 템플릿

개선된 PR title

기존 PR 템플릿에 있던 PR 타입 체크박스로 인해 PR 목록에서 작업 사항이 남은 것처럼 표시되어 혼동을 줄 수 있어 과감히 체크박스를 삭제하고 label로 PR 타입을 명시했다.

그리고 어떤 작업을 진행했는지 PR 제목만으로는 알 수 없었으며 이슈를 확인해야만 알 수 있었기에 PR 제목에 작업에 대한 간략한 설명을 추가했다.

또한 개선된 PR 템플릿에는 세 가지의 PR 목적 관점에서 필수로 들어가야 하는 내용들로 변경했다.

• 변경사항 관점
  • 구체적인 코드 변경 사항
  • 변경 이유
  • 구현 방법에 대한 고민
  • 테스트
• 코드 리뷰
  • 중점 리뷰 요청사항
  • 코멘트 (선택)
• 코드 병합
  • merge할 경우, 영향 범위
  • PR 반영 브랜치

 

작은 PR로 만들기

기존에는 많은 commit을 한꺼번에 PR로 요청해 리뷰어의 부담을 줄 수도 있다는 생각이 들어 새로운 화면이나 feature를 만들 경우 이슈를 여러 개로 쪼개고 여러 개의 PR로 작업하는 규칙을 만들었다.

각 이슈들에 대한 작업을 진행하면 PR은 이슈 개수만큼 만들어지며 feature branch를 base로 해서 그 아래 sub branch로 만들어 feature branch로 merge하는 전략으로 진행하기로 했다.

또한 PR 코드는 300줄 미만으로 유지하는 규칙을 만들었다. 

Cisco 시스템 프로그래밍 팀의 연구에 따르면 300줄에서 400줄 정도의 코드를 60분에서 90분 동안 리뷰하면 70~90%의 결함을 발견할 수 있다고 하기에 적합한 PR 사이즈는 300줄이라고 판단되어 최대한 300줄 미만으로 PR을 만들기로 했다.

 

결론

이번 기회에 좋은 PR이란 무엇인지 고민해보면서 PR도 결코 작은 것이 아니라고 생각됐다. 결국엔 PR도 하나의 글이므로 독자(리뷰어나 미래의 나)를 위해 읽기 쉽게 작성해야 한다는 것을 깨달은 좋은 경험이었다.

 

---

참고 자료

https://blog.banksalad.com/tech/banksalad-code-review-culture/

코드 리뷰 in 뱅크샐러드 개발 문화 | 뱅크샐러드

https://blog.jbee.io/essay/%EC%BD%94%EB%93%9C+%EB%A6%AC%EB%B7%B0%EC%9D%98+%EB%AA%A9%EC%A0%81%EC%9D%80+%EC%84%B1%EC%9E%A5%EC%9D%B4%EC%96%B4%EC%95%BC+%ED%95%9C%EB%8B%A4

코드 리뷰의 목적은 성장이어야 한다 - jbee.io

https://medium.com/prnd/%ED%97%A4%EC%9D%B4%EB%94%9C%EB%9F%AC-%EA%B0%9C%EB%B0%9C%ED%8C%80-%EB%AA%A8%EB%91%90%EA%B0%80-%ED%96%89%EB%B3%B5%ED%95%9C-%EA%B0%9C%EB%B0%9C-pr%EA%B4%80%EB%A6%AC-%EB%B0%A9%EB%B2%95-7%EA%B0%80%EC%A7%80-1d4cd5d091f0

헤이딜러 개발팀 모두가 행복한 개발/PR관리 방법 7가지

https://engineering.linecorp.com/ko/blog/effective-codereview

효과적인 코드 리뷰를 위해서