실용적인 코드 리뷰 문화 엿보기 (feat. 뱅크샐러드)

GitHub Pull-Request 관련하여 뭔가 만들어보려고 하다보니

`코드리뷰`를 어떻게 하는 것이 효율적인 것인지에 대해서 알아보게 되었다.

 

그러다가 알게된 "코드리뷰 in 뱅크샐러드 개발 문화"라는 포스팅 !!!

왜 진작 알지 못했을까!? 라는 탄식이 나오게 하는 너무나 유용한 내용이었다.

  - https://blog.banksalad.com/tech/banksalad-code-review-culture/#커뮤니케이션-비용을-줄이기-위한-pn-룰

 

 

꼭, 반드시 원본글을 완독해보시기를 강력하게 추천을 드린다.

아래 포스팅은 내가 감동한 부분에 대한 이야기만 조금 남겨보도록 하겠다.

 

Communication: Sync vs. Async

회사에서 일을 하다보면 참으로 많은 인터럽트 상황이 발생하게 된다.

주로 발생하는 것은 메일, 메신저, 전화, 문자 그리고 직접적으로 찾아오는 사람 방문(대면) 등

 

`개발자의 몰입 비용`에 대해서도 한 번 정리해보고 싶긴 한데, 지금은 소통 방식에 대해서 집어보도록 하자.

 

소통하는 방식을 조금 다르게 분류해보면 `동기(sync)`와 `비동기(async)`으로 나누어 볼 수도 있다.

그 중에서 개인 업무 시간을 보장할 수 있는 소통 비용이 저렴한 방식은 `비동기 소통`이다.

 

개인의 업무 시간, 특히 집중을 깨뜨리지 않기 위해서는 `비동기 소통(async communication)`이 중요하다.

그래서, 뱅크샐러드에서도 기본적인 소통 방식을 `비동기 소통`을 하도록 권장하고 있고

그 연장선에서 코드리뷰 도구로 GitHub Pull-Request를 선택했다고 한다.

 

나도 회사에서 업무할 때

PR을 떠나 개발자들이 더 잘 집중할 수 있도록 `비동기 방식의 소통`을 기본 정책으로 하는 것에 대해 많이 고민해봐야겠다.

 

 

작은 PR(Pull-Request)

코드 리뷰를 할 때에 매번 언급되면 `작은 PR`.

누구나 알고있지만 누구나 잘 지키기 어려운 ... 항상 있는 숙제 같은 느낌의 정책이다.

 

나도 회사에서 이를 지켜보려고 하지만 정말 쉽지 않다.

그러다가 저 블로그 포스팅에서 얻은 힌트 ~~~ !!!

 

예외를 명시적으로 두자 !!!

 

Initial Commit, Test Code 등 큰 덩치로 들어오는 경우에 대해

공식적으로 예외 규칙을 두게 되면 좀 더 자연스럽에 PR을 운영할 수 있을 것 같다.

 

1,000 line 이후의 작은 PR을 유지하면 리뷰 병목을 해소할 수 있다.

 

 

`Low Context` Communication

`Low Context`는 번역될 때 다양하게 표현된다.

  - 저 문맥

  - 저 맥락

  - 저 배경

 

어떻게 번역되더라도 사실 잘 와닿지는 않는다.

 

Low가 있다는 것은 반대로 High도 있다는 것인데,

`High Context`는 조금 돌려말하는 것을 의미하고 `Low Context`는 직설적으로 표현하는 것을 의미한다고 한다.

 

`Code Review`를 요청할 때에는 `Low Context`로 설명을 해줘야 한다는 것이다.

 

상대방이 `이 정도는 알거야`라고 생각하지도 말고, 애둘러 표현하거나 추상적으로 표현하지도 말고

그냥 정확하게, 직설적으로, 최대한 구체적으로 표현(소통)하는 것이 효과적이다.

 

파트원들에게 꼭 하고 싶은 중요한 rule인 것 같다.

 

 

`Pn` 룰

개인적으로 가장 감동을 받은 정책이다.

처음 본 것도 아니고, 모르던 것도 아니고, 안해본 것도 아닌데 그럼에도 신선했다 !!!

 

P1: 꼭 반영해주세요 (Request changes)
  - 중대한 코드 수정이 반드시 필요하다고 판단되는 경우. 합리적인 의견을 통해 리뷰어 설득 필요

P2: 적극적으로 고려해주세요 (Request changes)
  - 수용하거나 만약 수용할 수 없는 상황이라면 적합한 의견을 들어 토론할 것을 권장

P3: 웬만하면 반영해 주세요 (Comment)
  - 수용할 수 없는 상황이라면 이유 설명 또는 향후 반영 계획 명시

P4: 반영해도 좋고 넘어가도 좋습니다 (Approve)
  - 무시 가능. 고민 추천

P5: 그냥 사소한 의견입니다 (Approve)
  - 아무런 의견을 달지 않고 무시해도 괜찮음

 

 

뱅크샐러드에서는 단지 PR에만 사용하는 것이 아니라

Slack 대화할 때 또는 평상시에도 Prefix로 자연스럽게(문화) 사용하고 있단다.

 

우와!!!!

부서에 꼭 반드시! 빠르게! 적용하고 싶은 rule 이다.

 

 

`D-n` 룰

`Pn 룰`만큼은 아니지만, 그래도 너무나 좋은 정책이라고 여겨지는 rule이다.

 

D-0 (ASAP)
  - 긴급한 수정사항으로 바로 리뷰해 주세요.
  - 앱의 오류로 인해 장애가 발생하거나, 빌드가 되지 않는 등 긴급 이슈가 발생할 때 사용합니다.

D-N (Within N days)
  - “Working Day 기준으로 N일 이내에 리뷰해 주세요”
  - 일반적으로 D-2 태그를 많이 사용하며, 중요도가 낮거나 일정이 여유 있는 경우 D-3 ~ D-5 를 사용

 

우와 ... 얼마나 명확한 소통 방식인가 !!!

뱅크샐러드에서는 Prefix 방식이 아니라 Tag 방식으로 활용하고 있단다.

 

 

 

그렇게 어렵지도 않고, 복잡하지도 않은 `리뷰 정책`인데,

제대로 정착만 된다고 하면 정말 상당히 꽤 효율적으로 pull-request를 수행할 수 있을 것이라 믿음이 생긴다.

 

간단히 요약해보았는데, 관심이 있으신 분은 원본 글을 꼭 찾아가서 읽어보시기 바랍니다!!!