LeChuck

코드 리뷰에 적합한 PR 작성하기

·3 min to read

코딩 스타일을 통일해야만 한다

A,B 두 명의 개발자가 서비스의 기반을 열심히 다졌다. 이들은 논의를 통해 나름의 코딩 스타일을 정의해서 문서화하였다. 또한 코드 리뷰를 통해 서로의 코드를 교정해주곤 했다. 하지만 바쁜 일정상 코드 리뷰는 사치에 가까웠다. 따라서 공통 로직에는 가급적 코드 리뷰를 시행하여 어느 정도의 일관성이 갖춰질 수 있었지만, 각자가 작업한 개별 로직은 자유개척지이고 무법지대였다. 이후 A는 퇴사하고 C가 합류했다.

C는 기존 코드에서 아쉬운 부분이 많았다. 새로운 기능들을 열심히 쳐내면서, 간혹 숨 돌릴 시간이 주어지면 리팩토링했다. 문서를 참고하며 기존 프로젝트에 작성된 컨벤션을 최대한 준수하려 노력했다. 리팩토링된 코드는 분명 기존 코드보다 간결하고 읽기 쉬웠기에, C는 이것을 '개선'으로 여겼다. B도 만족스러워 하는 눈치다. 코드 리뷰를 통해 직접적인 피드백을 받진 못했지만 설계 과정, 그리고 작업 완료 이후에 구두로 공유하며 간접적인 피드백을 받았다.

이게 정말 '개선'이었을까? 사실 프로젝트의 관점에서 보았을 때 C가 더 나아졌다고 자부한 작업물은 '또 하나의 일관성 없는 코드 뭉치'에 불과했다. 그리고 이러한 코드 뭉치는 시간이 지날수록 계속해서 늘어났다. 아마 베테랑 개발자가 작성한 웰메이드 코드일지라도 '또 하나의 일관성 없는 코드 뭉치'에 불과할 것이다. A,B가 쌓아온 만큼 C도 열심히 쌓았다. 그리고 D가 팀에 합류했다. C는 악순환을 끊어내야 겠다고 다짐한다.


코딩 스타일에 대해 논쟁하는 이유라는 글에 따르면, 코드 스타일이 통일되지 않은 기업에서는 이른바 코드 스타일 전쟁이 발생한다고 한다. 이는 오래 전에 코드 스타일에 대한 협의를 중단한 개발진 사이에서, 변경 요청이 들어오면 이를 핑계삼아 다른 개발자가 작성한 기존의 코드를 자신의 입맛대로 경쟁하듯 변경하는 상황을 일컫는다. 스타일 전쟁은 팀의 긴장감을 부추기고 팀원들의 사기를 저하시킨다.

  • 이 코드는 누가 작성했고, 저 코드는 누가 작성한지를 한 눈에 분별할 수가 있다면 그건 비극이다.
  • 스타일 가이드를 따르지 않는다는 선택지는 없다. 어떤 조직을 가도 스타일 가이드가 존재하기 때문이다.
  • 이 이야기의 교훈은 코드가 자신들의 방식대로 작성되어 있는것보다 모든 코드가 비슷한 스타일로 되어 있는 것이 훨씬 중요하다는 것.

코드 리뷰 프로세스를 정착시키면 코드 스타일을 효과적으로 통일할 수 있을 것 같았다.

코드 리뷰를 통해 코딩 스타일을 통일하자!

작업단위를 잘게 쪼개자

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

커밋, PR을 최대한 작은 단위로 관리하자 (단일책임원칙을 적용하라) Pasted image 20240501173023.png 하나의 커밋에 하나의 책임만 부여된 예시 Pasted image 20240501173306.png 하나의 이슈가 곧 하나의 PR이 되게 하라

  • HDA-1954라는 하나의 base issue 정의
  • base issue에서 파생되는 여러개의 sub issue를 정의
  • 각각의 issue가 모두 PR이 된다
  • feature/HDA-1954, feature/HDA-1954-1956
  • 1956(feature-sub)에서 1954(feature)로 PR, 1954에서 dev로 PR

PR 직전에 커밋을 정리해서 올리자 (rebase)

GIT 형상 관리 + 작업 단위와 PR 코드리뷰 + 협업 잘 하는 개발자

bad case

  • 커밋1에 User 수정, 이후 커밋에도 계속 User가 수정되어서 추적이 어려움 Pasted image 20240428183647.png

Pasted image 20240428185711.png

  • 작업할 땐 생각없이 커밋을 작성하고 쌓았더라도, PR 올리기 전에 reset, rebase를 통해서 PR에 최적화된 방식으로 정리해야 한다.