공통시스템개발팀 코드 리뷰 문화 개선 이야기

Dec.23.2021 배대준
clipboard facebook twitter

Culture

안녕하세요. 공통시스템개발팀 배대준입니다.

Merge Request(Pull Request)를 생성했는데 리뷰어는 묵묵부답이고 직접 요청하자니 업무를 방해하는 건 아닌가 걱정하신 적이 있으신가요?

작년에 저희도 Merge Request(이하 MR)가 쌓이면서 같은 고민을 시작했고 지난 1년간 코드 리뷰를 잘하기 위해서 코드 리뷰 문화를 개선한 과정을 이야기하려 합니다.

1년 전..

MR은 쌓여가는데 리뷰가 늦어서 직접 요청하는 경우가 자주 있었습니다.

그에 따라 리뷰이는 리뷰를 기다리고 리뷰어는 수많은 MR을 확인하느라 고통받고 있었습니다.

리뷰가 쌓이는(늦는) 원인

  • 리뷰어가 이해하기 쉽게 MR 내용 작성
  • 같은 파트원의 승인 이후에 머지
  • MR, 리뷰 코멘트 발생 시 슬랙으로 알람

기존 코드 리뷰 문화는 이랬습니다. 크게 부족해 보이는 부분은 없어 보였는데 왜 쌓이는지 고민했고 결과적으로 리뷰어가 리뷰에 손이 가지 않기 때문에 리뷰가 쌓이는 것이라고 결론 내렸습니다. 따라서 리뷰어가 적은 노력으로 코드 리뷰를 잘할 수 있는 환경을 갖추기로 했습니다.

적은 노력으로 리뷰 잘하기

1. MR 템플릿

기존에는 MR 작성 규칙이 없어서 양식이 일관되지 않았고 때로는 설명이 부족한 경우도 있었습니다.

따라서 일관된 방식으로 작성하도록 MR 템플릿을 만들었습니다.

완성된 MR 템플릿

# 해결하려는 문제가 무엇인가요?
*

# 어떻게 해결했나요?
*

## Attachment
* 이번 MR 의 Front 동작을 이해를 돕는 GIF 파일 첨부!
* 리뷰어의 이해를 돕기 위한 모듈/클래스 설계에 대한 Diagram 포함!

작성 예시

# 해결하려는 문제가 무엇인가요?
* TS2305: Module '"react-router"' has no exported member 'useHistory'. 에러를 내면서 빌드가 깨집니다.
  다른 모듈에 의해 react-router 버전이 5 -> 6으로 올라간 게 문제입니다.

# 어떻게 해결했나요?
* 사용하는 react-router의 버전을 package.json에 명시합니다.

MR 템플릿을 적용한 이후로 전체적으로 MR 내용도 좋아졌지만 리뷰이도 MR 작성에 고민이 줄어들어 편해졌습니다.

2. 매일 아침 MR 목록 알람


(당시 스타트업 드라마에 심취한 팀원의 멘트 선택)

클라우드스토리지개발팀에서 매일 아침 MR 목록을 슬랙 알람으로 받는 걸 보고 저희 팀도 이용하기 시작했습니다.

방법은 python-gitlab을 이용해서 파이썬 코드를 작성 후 배치를 등록했습니다.

3. 칸반 리뷰 맥스

그러나 알람만으로는 충분하지 않았고 칸반에서 리뷰 MAX 초과 시 모든 팀원이 업무를 중단하고 리뷰 하기로 규칙을 정했습니다.

또한 업무에 집중하기 위해 진행 중인 업무는 1인당 1개로 결정했습니다.

4. 코드 리뷰 규칙 문서 추가

MR 템플릿으로 어느 정도 통일은 갖췄으나 MR 크기와 코멘트 방식이 사람마다 달라서 추가 규칙을 문서로 작성했습니다.

문서 역시 클라우드스토리지개발팀에서 이미 잘 작성해 놓아서 저희 상황에 맞게 변경했습니다. (클라우드스토리지개발팀 매번 감사합니다)

리뷰이 규칙

리뷰어가 적은 노력으로 코드 리뷰를 잘할 수 있게 아래 규칙을 신경 써주세요.

코드 리뷰의 설명은 최대한 자세하게 작성되어야 합니다

  • 리뷰 작성자는 본인이 알고 있는 사항을 리뷰어들도 알고 있을 것이라는 가정을 버리고 코드 리뷰에 대한 충분한 문맥이 전달될 수 있도록 코드 리뷰 설명을 자세히 작성해 주세요.
  • MR Template에 맞춰서 작성해 주세요.

작은 MR을 유지하세요

  • 리뷰어들이 코드 리뷰에 들어가는 시간을 줄이고 최대한 많은 버그들을 미리 발견해낼 수 있도록 코드 리뷰의 크기는 삭제를 포함해서 최대 300줄 미만으로 유지해 주세요.
  • 적합한 MR 단위는 어떻게 되나요?
    • 하나의 티켓에서 여러 개의 MR을 작성해도 됩니다.
    • 리팩토링 작업은 분리해 주세요.
    • 작게 쪼개기를 참고해 주세요.

라벨로 코드 리뷰의 우선순위를 표시하세요 (D-n 규칙)

  • 코드 리뷰가 완료되어야 하는 시점을 D-N 형식의 라벨로 코드 리뷰에 추가해 주세요. 예를 들어, D-3 은 3일 이내에 코드 리뷰가 리뷰어에게서 확인되어야 한다는 의미입니다.
  • 이슈가 없는 코드 리뷰는 D-3 라벨로 표기하고 당장 긴급하게 시스템에 반영되어야 하는 사항은 D-0 라벨로 표기해서 모든 리뷰어가 긴급하게 코드 리뷰를 할 수 있도록해 주세요.
  • 매일 라벨을 업데이트해주세요.
  • D-0이 된 다음 날까지 어느 리뷰어도 코드 리뷰를 시작하지 않았다면 리뷰 작성자는 스스로 해당 변경 사항을 코드 베이스에 반영(Merge)할 수 있습니다.

최소 한 명의 리뷰어에게 리뷰를 받아야 합니다

  • 같이 프로젝트를 진행하는 팀원들의 코드 리뷰를 받습니다.
  • 필요하면 같이 프로젝트를 진행하지 않는 팀원을 리뷰어로 할당해서 요청합니다.

피드백을 반영하면 코멘트를 남겨주세요

피드백에 변경한 내용이 무엇인지 리뷰어에게 알려주세요.

리뷰어 규칙

코드 리뷰의 코멘트에 코멘트를 강조하고 싶은 정도를 표기하세요 (Pn 규칙)

리뷰어는 코드 리뷰의 코멘트에 코멘트를 강조하고 싶은 정도를 Pn 규칙에 맞춰서 표기해 주세요:

P1: 꼭 반영해 주세요 (Request changes)
P2: 적극적으로 고려해 주세요 (Request changes)
P3: 웬만하면 반영해 주세요 (Comment)
P4: 반영해도 좋고 넘어가도 좋습니다 (Approve)
P5: 그냥 사소한 의견입니다 (Approve)

리뷰 작성자를 칭찬해 주세요

리뷰어는 코드 리뷰에 별달리 코멘트할 내용이 없다면 변경 사항을 작업하기 위해 수고한 리뷰 작성자를 칭찬하는 코멘트를 남겨주세요.

참고 문서

참고 문서를 보시면 보다 자세한 내용을 확인하실 수 있습니다.

5. MR 리스트 알람에 d-n 표기 추가

MR 리스트 알람에 D-n 표기를 추가했습니다.

6. d-n 라벨 자동화

기존에는 d-n 라벨을 매일 아침 리뷰이가 직접 변경했습니다. 그러다보니 어느 순간부터 잘 사용하지 않게 되어 자동화의 필요성을 느꼈습니다. 이것 또한 python-gitlab을 이용해서 파이썬 코드 작성 후 배치를 등록했습니다.

규칙

  • 라벨이 없으면 D-5 추가
  • 하루씩 감소 ex) D-5 → D-4으로 변경
  • D-0이 되면 리뷰어 승인 없이 머지할 수 있게 approve rule 제거

7. pre-commit으로 포맷팅 자동화

pre-commit은 이전부터 사용했지만 유용하게 쓰고 있어서 내용에 추가했습니다.

현재 사용하고 있는 언어의 린터인 ktlinteslint git hook pre-commit을 이용해서 커밋하기 전에 실행합니다.

또한 MR 생성시 CI에서 lint를 실행하여 컨벤션을 지켰는지 추가로 확인합니다.

#!/bin/sh

CHANGED_FILES="$(git --no-pager diff --name-status --no-color --cached | awk '{ print $2 }')"
CHANGED_KOTLIN_FILES="$(git --no-pager diff --name-status --no-color --cached | awk '$1 != "D" && $2 ~ /\.kts|\.kt/ { print $2}')"

git_add_diff_files() {
    for file in $CHANGED_FILES; do
        git add "${file}"
    done
}

# ktlint
./gradlew --quiet ktlintFormat -PinternalKtlintGitFilter="$CHANGED_KOTLIN_FILES"

if ! ./gradlew --continue ktlintCheck -PinternalKtlintGitFilter="$CHANGED_KOTLIN_FILES"; then
    exit 1
fi

# eslint
cd {front directory}
yarn run lint-staged
cd {root directory}

# 린트 실행 후 변경된 파일 다시 git add
git_add_diff_files

결과적으로 현재 상태

  • 매일 아침 9시 30분(월요일은 오후 1시)에 d-n 라벨을 수정 후 슬랙으로 알람 발송
  • MR 단위는 최대한 작게
  • 일관된 양식으로 작성된 MR
  • 리뷰가 쌓이면 업무 중단 후 리뷰 먼저 처리

현재는 이렇게 일하기 때문에 ‘쌓이는 리뷰’가 많이 줄었습니다. 또한 MR은 많지만 바쁠 때는 중요한 MR을 먼저 리뷰하기 때문에 효율적으로 일할 수 있게 되었습니다.

마치며

개선 과정에서 다른 팀, 회사, 개인 블로그를 참고하며 많은 도움이 되었고 저희 팀 리뷰 문화 또한 공유하면 좋을 거 같아서 글을 쓰게 되었습니다.

코드리뷰는 중요하지만 때로는 여러 가지 이유로 소홀해질 때가 있습니다. 저희는 리뷰어가 적은 노력으로 리뷰에 집중할 방법을 계속해서 생각했고 결과적으로 1년 전보다 코드 리뷰를 효율적으로 하고 있습니다. 이런 저희팀에서 함께 좋은 제품을 만들어갈 분들은 공통시스템개발팀 채용공고 많은 지원 부탁드립니다.