모두 개발자로만 이루어진 구성원들과 토이프로젝트를 할 때에는 git hub 에서 issues, Projects 칸반보드도 사용했었는데, 회사에서는 비개발자 PM, 기획자, 디자이너 님들과 작업 상황 공유 등 다른 툴로 협업이 필요하여 Pull Requests 만 사용하고 있다.
이번에 본격적으로 신입 PR 리뷰를 해드리는 역할도 맡게 되어서
PR 리뷰에 시간은 최대한 적게 뺏기면서 도움이 되는 리뷰를 드릴 수 있도록 효율적인 PR 리뷰를 위한 방법들을 찾아보고, 그동안 해왔던 방식도 정리해 봤다.
사용중인 PR 템플릿
기본 템플릿은 다음과 같이 사용하고 있으며, PR 에 따라 필요 사항들을 추가하거나 삭제하여 사용하고 있다.
# What is this PR?
-
# Changes
-
-
-
# Test Check List
- [ ] check 1
- [ ] check 2
- [ ] check 3
# Screenshots or Video
| 비고 | 수정 전 | 수정 후 |
| ---- | -------- | -----|
| img | | |
pr merge 를 위한 branch rule set
main branch 에는 rule set 을 지정하여 PR 을 통해서만 merge 할 수 있고, pr 에 달린 코멘트들이 모두 해결되어야 merge 할 수 있도록 branch rule set 을 지정해 놓았다.
- private 레포지토리인 경우 GitHub Team 이거나 Enterprise 깃헙이어야 rule set이 적용된다.
Settings → Branches → Add branch ruleset 에서 적용할 수 있다.
husky 를 이용해 컨벤션 자동화
lint, prettier 와 같이 컨벤션 리뷰에는 최대한 시간을 뺏기지 않기 위해, 자동으로 맞춰지도록 husky 를 적용해 놓았다.
- commit 전 prettier 검사하여 적용
- push 전 tsc 검사, lint 검사하여 통과 되어야만 push 가능
⛑️ 기본적인 코드 포맷 외에도, 여전히 component, container, pages 단위, 동작 로직 함수는 어느 단위에서 처리할 것인지,아키텍처 구성 등에 대해서 리뷰어와 리뷰이의 의견이 맞지 않아 논의에 꽤 시간이 소요되었다. 이에 대해 회사의 컨벤션을 정하거나, 적어도 프로젝트 단위에서 명확히 컨벤션을 정하고 들어가야할 필요성을 느꼈다
깃헙 액션을 활용해 build 검사 자동화
배포는 aws amplify 를 통해서 merge 시 자동 배포 되고 있어서,
github actions 에서는 build 통과 CI 만 구축해 놓았다.
workflow 에 대해서는 아직 자세히 알지 못해서 next js build 및 배포를 위해 만들어져 있는 workflow 파일을 gpt 에게 물어가며 만들어서 등록했다.
⛑️ build 검사 외에도 프론트엔드에서 필요한 ci 에는 어떠한 것들이 더 있을 지 다른 기술 블로그들을 보면서 좀 더 공부해서 추가해봐야겠다.
- 도입해볼 것 : toss.tech - GitHub Actions로 개선하는 코드 리뷰 문화
git hub actions 로 코드 리뷰어를 지정하고 슬랙봇으로 알림 & 리마인더 보내기
다른 회사들의 코드 리뷰 문화 참고
기술 블로그에 올라온 다른 회사들의 코드 리뷰 문화들 중 우리 회사에서도 적용 가능할 것 같은 것들을 몇 가지 모아봤다.
출처 :
kakao tech-효과적인 코드리뷰를 위한 리뷰어의 자세
리뷰 요청자의 준비물
- 제품 기능 기획
- 기술적인 구현 계획
- 현재 PR에서의 변경점
- 예상 / 구현된 디자인
- 어떤 내용들을 테스트했는지 / 해야 하는지 목록
코드 리뷰에 임하는 자세
코드 리뷰 과정에서 이루어지는 것
- 일관된 아키텍처를 유지하고 있는지
- 다른 해결 방법에 대한 의견 제시
- 버그가 발생할 수 있는 가능성 제시
- 기술적인 지식, 노하우 공유
- 히스토리 전달
리뷰의 5가지 규칙
1. 왜 개선이 필요한지 이유를 충분한 설명해 주세요.
2. 답을 알려주기보다는 스스로 고민하고 개선 방법을 선택할 수 있게 해주세요.
3. 코드를 클린 하게 유지하고, 일관되게 구현하도록 안내해 주세요.
4. 리뷰 과정이 숙제검사가 아닌 학습과정으로 느낄 수 있게 리뷰해 주세요.
5. 리뷰를 위한 리뷰를 하지 마세요. 피드백 할 게 없으면 칭찬해 주세요.칭찬 피드백에 대한 예시
- 코드량이 적당해서 읽기 편하네요.
- 많은 고민이 코드에서 엿보이네요.
- 성능에 아주 유리한 코드라고 생각되네요.
- 전에 코드보다 훨씬 좋아진 거 같네요.
- 예외 처리가 꽤 꼼꼼해서 좋네요.
- 함수, 변수명이 직관적이어서 좋네요.
커뮤니케이션 비용을 줄이기 위한 Pn 룰
뱅크샐러드 기술 조직은 코드 리뷰의 코멘트에 Pn 룰을 사용하여 리뷰어가 코멘트를 강조하고 싶은 정도를 표현합니다.
P1: 꼭 반영해주세요 (Request changes)
리뷰어는 PR의 내용이 서비스에 중대한 오류를 발생할 수 있는 가능성을 잠재하고 있는 등 중대한 코드 수정이 반드시 필요하다고 판단되는 경우, P1 태그를 통해 리뷰 요청자에게 수정을 요청합니다. 리뷰 요청자는 p1 태그에 대해 리뷰어의 요청을 반영하거나, 반영할 수 없는 합리적인 의견을 통해 리뷰어를 설득할 수 있어야 합니다.
P2: 적극적으로 고려해주세요 (Request changes)
작성자는 P2에 대해 수용하거나 만약 수용할 수 없는 상황이라면 적합한 의견을 들어 토론할 것을 권장합니다.
P3: 웬만하면 반영해 주세요 (Comment)
작성자는 P3에 대해 수용하거나 만약 수용할 수 없는 상황이라면 반영할 수 없는 이유를 들어 설명하거나 다음에 반영할 계획을 명시적으로(JIRA 티켓 등으로) 표현할 것을 권장합니다. Request changes 가 아닌 Comment 와 함께 사용됩니다.
P4: 반영해도 좋고 넘어가도 좋습니다 (Approve)
작성자는 P4에 대해서는 아무런 의견을 달지 않고 무시해도 괜찮습니다. 해당 의견을 반영하는 게 좋을지 고민해 보는 정도면 충분합니다.
P5: 그냥 사소한 의견입니다 (Approve)
작성자는 P5에 대해 아무런 의견을 달지 않고 무시해도 괜찮습니다.
리뷰 우선순위 판단을 돕는 D-n 룰
리뷰어는 리뷰를 요청하는 시점에 PR 이 Merge 되어야 하는 일정을 공유하여 리뷰어가 Working day 안에서 스스로 우선순위를 결정하고 개발에 집중할 수 있는 시간을 환경을 만들어 나가고 있습니다.
D-0 (ASAP)
긴급한 수정사항으로 바로 리뷰해 주세요. 앱의 오류로 인해 장애가 발생하거나, 빌드가 되지 않는 등 긴급 이슈가 발생할 때 사용합니다.
D-N (Within N days)
“Working Day 기준으로 N일 이내에 리뷰해 주세요”
일반적으로 D-2 태그를 많이 사용하며, 중요도가 낮거나 일정이 여유 있는 경우 D-3 ~ D-5 를 사용하기도 합니다.
화해 백엔드 플랫폼, 코드 리뷰 템플릿
[L0-리뷰불가]
- PR Description, 테크 스펙 등 어떤 작업을 했는 지 충분한 설명이 없는 경우
- 변경 작업이 너무 커서 리뷰가 어려운 경우
- 설계가 잘못되어 전체적으로 재작업이 필요하다고 판단해 리뷰를 진행할 수 없는 경우
[L1-변경요청]
- 기능 결함, 심각한 퀄리티 저하, 팀 컨벤션 위반 등 반드시 머지 전에 변경이 필요한 경우
[L2-변경협의]
- 가급적 머지 전에 변경되었으면 좋겠지만, 배포 후 후속작업으로 바로 진행이 된다면 괜찮다고 생각하는 경우
- 작성자가 의견에 동의하지 않는다면 토론 필요
[L3-중요질문]
- 궁금증이 해소되어야 정확한 리뷰/피드백이 가능하다고 판단하는 경우
[L4-변경제안]
- 제시한 방안이 더 좋다고 생각하지만 작성자 판단에 맡겨도 무관한 경우
[L5-참고의견]
- 더 좋은지는 모르겠으나 다른 방법 제시 혹은 참고할만한 내용인 경우
리뷰 코멘트 템플릿을 🔗 Saved replies 에서 지정하여 단축키만으로 사용할 수 있다.
