[WHY 시리즈 1] E.012 — 두 번째 Pull Request 이야기

Mijeong (Rachel)
6 min readNov 17, 2019

--

‘코드와 서비스 만들기를 좋아하는 소프트웨어 엔지니어’로 일하고 있다. 엔지니어는 문제를 해결하는 사람이다. 문제를 해결하는 과정에는 여러 가지 선택이 존재할 수 있다. 가능한 최선의 결과를 위해, 항상 내 선택의 Why에 답할 수 있는 사람이 되고자 한다. 매주, 한 주 동안 내가 결정한 선택의 Why에 대해 정리하고자 한다.

[WHY 시리즈 1]E.007 — Pull Request를 통해 잡은 오류들 글에서 Pull Request 리뷰를 통해 배포 전 발견한 오류들을 이야기했다. 약 8개월이 지났고, 플랫폼 팀의 동료들도 늘어나면서 그동안 추가 Pull Request가 참 많이 쌓여있었다. 이미 merge된 수많은 Pull Request의 리뷰를 다시 살펴보다가 공유해도 좋을 내용을 발견했다. 몇백개의 Pull Request를 전부 자세히 살펴보지는 못했지만, 몇 가지 내용을 공유하고자 한다. 이번 주제에 대해서는 WHY 대신 WHAT을 던져보는 게 더 적절하겠다.

What?

리팩토링 기회

Code refactoring is the process of restructuring existing computer without changing its external behavior.

Pull Request 리뷰 (혹은 코드 리뷰)를 진행할 때, 유의미하며 그만큼 기대하기 어렵다고 판단한 효과 중 하나가 리팩토링이라 생각한다. 우리 팀에서는 최근 리팩토링 효과를 얻은 Pull Request 리뷰 사례가 있다.

우리 프로덕트는 기존에 쿠폰 코드를 이용한 프로모션만 존재했다. 최근 새로운 유형의 프로모션이 추가되면서 가격 필드에 대한 정책의 복잡도가 높아졌다. 급하게 반영한 코드의 복잡도도 함께 높아졌다. 무사히(?) 새로운 유형의 프로모션을 릴리즈하고 한동안의 시간이 흘렀다. 얼마 지나지 않아 프로모션 유형에 추가 요구사항이 생겼고, 가격 관련 코드의 로직 및 필드의 상태가 매우 심란한(?) 상황이 되어 Pull Request가 생성됐다. 프로모션 유형의 다양한 조합에 대한 테스트는 문제없이 통과했지만, 리팩토링의 신호가 강렬하게 느껴졌다. Reviewer였던 나는 Author였던 동료와 함께 종이와 펜을 꺼내 들고 약 2시간 가까이 리팩토링을 시도했다. 목적은 1) 정말 필요한 로직만 남기며 2) 의미가 더욱 명확하게 전달될 수 있도록 개선하는 것이었다. 동료와 나는 수많은(?) 가격 필드의 의미를 확인하며 목적을 달성했다. 동료가 생성한 Pull Request는 동료의 코드가 아닌 우리의 코드다. 내가 직접 작성한 코드에 언제든 영향을 줄 수도 있고, 또 언젠가는 내가 직접 수정을 해야 하는 코드일 수도 있다. 이런 마음가짐이라면 Pull Request 리뷰에 할애하는 시간을 아깝다고 생각할 수 없을 것이다.

리팩토링 전 코드.png
리팩토링 후 코드.png
사이좋은 동료와 나.png

네이밍 전쟁

Pull Request를 리뷰하면서 가장 많이 발견되거나 가장 자주 불편하다고 느껴지는 리뷰 중 하나가 네이밍이 아닐까 싶다. 각자의 취향이 반영되어 불필요한 리뷰 전쟁이 발생하지 않도록, 네이밍 리뷰에 대한 2가지 기준을 갖고 있다.

  • 주어가 무엇인지 확인하고 자연스러운 네이밍을 선택하자. 예를 들어, User model에 특정한 이유로 block된 사용자인지 여부를 확인하는 필드가 추가된다고 가정하자. blockXXX 보다는 isBlockedByXXX 이라는 네이밍을 제안할 것이다. 풀어서 확인하면 (User) is blocked by XXX 은 자연스러운 문장이 되기 때문이다.
  • 의미가 돌직구로 전달되는 네이밍을 선택하자. 예를 들어, 특정 요청을 위해 사용되는 특정 필드의 값은 항상 ‘dish_’ 라는 prefix가 붙는다고 가정하자. const TYPE_DISH = ‘dish_’ 보다는 const PREFIX_DISH = ‘dish_’를 제안할 것이다.
2세를 위한 작명 요청만 기다리고있다.png
나는 단순하게 좋다 나는 1차원이다.png

불투명한 미래

기준이 없는 네이밍 리뷰 전쟁보다는 덜 하겠지만, Pull Request 리뷰 중 불편하다고 생각되는 것 중 하나로 불투명한 미래에 대한 방어/개선 코드 제안이다. 프로덕트의 새로운 피처를 선정할 때 꼭 필요한 최소한의 스펙을 정의하려는 것처럼, 코드를 리뷰할 때에도 가능한 현재의 스펙을 기준으로 삼으려고 한다. 절대 앞을 내다보지 않는 리뷰를 하자는 말은 아니다. 예를 들면, 아직 정의되지 않은 스펙까지 고려한 리뷰는 지양하자는 의미이다.

영어를 써야만 하는 한국인 개발자의 고충.png

함께 공부하기

Pull Request를 리뷰하면서 얻을 수 있는 기대 효과 중 개인적으로 단연 좋아하는 점은 애매하게 알고 있던 것을 정확히 이해할 수 있는 기회를 발견할 수 있다는 것이다. 우리가 사용하고 있는 기술에 대한 애매함이든, 우리가 구현하고 있는 스펙에 대한 애매함이든 질문하고 답을 얻고 논의할 수 있는 기회를 찾을 수 있다.

우리는 Redis 특정 함수에 대한 스펙 싱크 중.png
외부 서비스 payload 크기도 함께 확인하는 우리.png

우리 팀의 지난 Pull Request 리뷰를 돌아보고 이렇게 몇 자 적어보니 조금씩 더 건강하고 의미 있는 결과를 만들어가고 있어서 기쁘다. 물론, 항상 기억해야 하는 점이 있다. 여전히 개선할 점은 많고, 개선하기 위해서는 동료들의 적극적인 참여가 필요하고, 동료들의 적극적인 참여는 그럴 수 있는 환경에서 나온다. 늘어나는 동료들과 함께 건강한 환경을 만들기 위한 고민도 요즘 내 고민의 많은 부분을 차지하고 있다. 잘하고 있는지는 모르겠지만!

베트남 사람 동료들에게 막걸리를 소개하는 중.png

--

--