코드 리뷰는 동료가 작성한 코드를 점검하고 피드백을 교환하는 과정입니다. 구성원 모두가 책임감을 가지고 새로 추가하거나 변경하는 코드를 검토하는 중요한 활동입니다. 이 문서는 코드 리뷰를 받거나, 코드 리뷰를 하는 사람이 알아야 할 지침을 제공합니다.
- 모든 코드는 코드 리뷰를 마친 후에 머지를 해야 합니다.
- 혼자가 아닌 함께 하는 일이기에 우리 모두는 코드 리뷰에 책임감을 갖습니다.
- 최소 2명을 필수 리뷰어(assigner)로, 팀(meshkorea/web-front-end)은 리뷰어(reviewer)로 지정합니다.
- 2명의 필수 리뷰어가 PR을 승인해야 머지할 수 있습니다.
- 인프라 작업이나 릴리즈 PR은 리뷰를 생략할 수 있습니다. 단, review skip 레이블을 붙여서 스프린트 리뷰 시간에 일괄 검토합니다.
- PR을 받았으면 하루 안에 답변을 주세요. 답변이 없으면 재촉하세요.
- 리뷰하기에 양이 많다면, 오프라인 리뷰를 요청하세요.
- 특별히 신경을 써야 할 리뷰 포인트는 코멘트를 남겨주세요.
- 자신의 취향을 주장하기 보다는 사실과 데이터를 가지고 의견을 교환해주세요.
코드 리뷰는 PR(Pull Request)을 만드는 것에서 시작합니다. 충분한 정보를 제공하는 PR은 리뷰어가 더 정확하고 신속한 리뷰를 할 수 있도록 돕습니다. 이는 팀의 생산성을 향상시킵니다.
PR의 목적을 잘 전달하기 위해서는 충분한 정보를 동료에게 제공해야 합니다. 코드 리뷰를 하는 동료는 내가 아는 것들을 모른다고 가정하고 아래의 내용을 잘 따라주세요.
커밋 메시지의 제목만 봐도 어떤 작업을 했는지 유추할 수 있어야 합니다. 제목은 글자수 제한이 있기 때문에 어떤 작업을 했는지 설명하기 어려울 때가 있습니다. 이럴 때는 제목에 간략히 작업 내용을 요약해서 적고, 상세한 내용은 본문에 작성을 해주세요.
자세한 내용은 커밋 컨벤션을 참고하세요.
PR의 본문에도 PR을 작성한 계기, 수정 방향, 관련 이슈 등을 충실히 적어야 합니다. 아래의 PR 템플릿에 맞춰서 본문을 작성해주세요.
<!--
제목은 `[(키워드)] (작업한 내용) - (PR 순서)` 로 작성해 주세요
키워드 예시: feat/qa/sentry 등
-->
## 설명
### 작업 티켓 or 동기
<!--
(Optional)
작성한 작업에 대한 티켓 번호나, 티켓이 없는 경우라면 작업을 왜 하게 되었는지 서술합니다.
-->
### 스크린샷
<!--
(Optional)
UI가 변경되었다면 사진이나 Gif를 추가해 주세요.
-->
### 작업 내용
<!-- PR 본문을 입력하세요. -->
### 주안점
<!--
(Optional)
리뷰 시에 유심히 봐주었으면 하는 부분을 설명합니다.
-->
## 연관 PR
<!--
(Optional)
이 PR과 연관되어있는 다른 PR을 기입합니다.
예) - meshkorea/prime-api-client#15 : 연관된 API 변경점
-->
## 연관 티켓
<!--
(Optional)
이 PR과 연관되어있는 에픽 링크, 연관되어있는 QA 이슈 등을 기입합니다.
예) - 에픽 티켓: PP-9999
-->
## 체크리스트
- [ ] 무엇을 변경했는지 충분히 설명하고 있나요?
- [ ] 새로운 기술을 사용했다면, 그 기술을 설명하고 있나요?
- [ ] 이해하기 어려운 비즈니스 로직에 관해서 부연 설명을 하고 있나요?
- [ ] 팀원 두명을 Assign하고, Review Request에는 web-front 팀을 추가했나요?
- [ ] 작업을 설명하는 Jira 티켓을 추가했나요?
- [ ] 작업 범위에 대해서 테스트를 작성하셨나요?
- [ ] 인프라 작업, 릴리즈 PR이라면, Review Skip 레이블을 추가했나요?
- 변경한 코드의 양이 적을수록 리뷰를 하기가 더 쉽습니다.
- 변경한 코드가 적을수록 버그가 있을 가능성이 낮습니다.
- 동료가 더 짧은 시간 동안에, 더 집중해서 리뷰를 할 수 있습니다.
- 코드 리뷰에서 받은 피드백을 적용하기가 더 쉽습니다.
- 하나의 PR에는 하나의 작업만 담겨 있어야 합니다. 하나의 PR에 여러 작업이 포함되면 리뷰를 하기가 어려워집니다.
- 주석, 커밋, PR 본문 등, 리뷰를 하는 데에 필요한 모든 정보를 충실히 제공해야 합니다.
- 새로운 함수나 API를 추가할 때는, 해당 API 또는 함수를 사용하는 코드도 PR에 포함시켜야 합니다.
사내 애플리케이션 설계 원칙과 일반적인 프로그래밍 설계 지침에 맞춰서 코드가 작성되어 있는지 확인합니다.
팀원 모두가 같은 도메인에서 작업하지 않기 때문에 도메인의 비즈니스 로직에 대해서는 리뷰를 하기가 어렵습니다. 하지만 코드 그 자체를 리뷰할 수는 있습니다. 이 섹션은 코드 그 자체를 리뷰할 때 중요하게 보아야 할 부분을 설명합니다.
- 중요한 값을 하드 코딩을 하고 있지는 않은가요?
- 주석이 코드가 하는 일이 아닌, 코드에 담긴 의도를 설명하고 있나요?
- 코드를 쉽게 이해할 수 있나요?
- 코드만 봐도 작성자의 의도를 이해할 수 있나요?
- 비즈니스 로직을 잘 설명하고 있는지 확인합니다.
- 올바른 변수명, 함수명, 클래스명을 사용했는지 확인합니다.
- 내가 이해할 수 없는 코드는 다른 동료도 이해할 수 없습니다. 내가 이해할 수 없다면 작성자에게 설명을 요구하거나 개선을 부탁합시다!
- 설정이 용이한가요? 코드의 특정 부분을 설정 파일(
.env)파일을 사용해서 설정을 변경하게 만들 수 있는지 확인합니다. - 스타일 가이드에 맞게 작성되어 있나요?
- 하나의 함수가 10 라인을 넘어간다면 너무 많은 관심사를 갖고 있는 건 아닌지 유심히 봐주세요.
- 중복된 코드는 없나요?
- 두 번 이상 반복해서 등장하는 코드는 함수나 클래스로 분리하는 걸 검토하세요.
- 예외 처리를 제대로 하고 있나요?
- 자원을 효율적으로 반환하고 있나요? 예)
useEffect의 clean up function에서 제대로 초기화를 했는지
- 새로운 기능을 추가하기 쉽게 작성되어 있나요?
- 함수나 클래스가 두 개 이상의 관심사를 갖고 있지는 않은지 확인해주세요.
- 모든 코드는 테스트 하기가 쉬워야 합니다.
- 큰 함수를 작은 단위로 나눠주세요.
- 클래스 간에 모킹하기 쉬운 인터페이스로 연결되어 있는지 확인해주세요.
- 테스트가 작성되어 있는지 반드시 확인을 합니다.
- 테스트가 작성되어 있다면 테스트 작성 가이드를 참고해서 개선할 부분이 있는지를 확인합니다.
- 다음과 같은 단계로 리뷰를 해보세요!
- 전체 코드를 훑어보기
- 코드 전체에서 일어나는 변경을 가볍게 훑어보세요.
- 의미가 없는 변경점이나 변경이 일어나서는 안 되는 곳이 있는지 확인합니다.
- PR의 주안점 확인
- 리뷰를 받는 사람이 제시한 주요 변경사항을 확인합니다.
- 주안점이 제시되어 있지 않다면 PR 작성자에게 설명을 요구합니다.
- 구조를 변경한 PR은 빠르게 피드백을 전달하고 동료에게 전파를 해야합니다.
- 놓치는 파일이 없도록 모든 부분을 꼼꼼하게 확인합니다.
- 전체 코드를 훑어보기
- 잘한 점이 있다면 칭찬해 주세요!
- 리뷰를 해야 할 코드의 양이 너무 많다면? 오프라인으로 리뷰를 요청하세요.
- 코드 리뷰는 요청을 받는 즉시 하는 걸 원칙으로 합니다.
- PR을 받았으면 하루 안에는 답변을 주세요. 상대방의 답변이 없으면 재촉하세요.
- 자리에 앉아서 다른 업무를 하기 전에 코드 리뷰를 하는 습관을 가져보아요.
- 출근 직후
- 점심 먹고 자리에 앉았을 때
- 회의나 화장실에 갔다가 돌아와서 업무를 시작하기 전에
코드 리뷰에 의견을 작성할 때는 커뮤니케이션에 특히 신경을 써야 합니다. 대부분의 리뷰를 텍스트로 진행하므로 오해가 생기기 쉽습니다. 정확한 사실을 부드럽게 전달하도록 노력해주세요.
동료가 작성한 코드는 나와 다른 관점에서 작성한 것일 수 있습니다. 리뷰를 하기 전에 작성한 의도를 먼저 물어봐주시고 동료의 의도를 이해하려고 노력해주세요.
우리는 팀으로 일을 합니다. 우리가 작성하는 모든 코드는 내가 아닌 팀의 자산입니다. 자신의 취향을 주장하기 보다는 협업을 하기에 좋은 코드를 찾는 데에 힘을 모아주세요.
권장 사례(Best Practice)를 설명한 문서나, 공식 문서 등을 활용해서 의견에 근거를 제시하세요.
A) 좋지 않은 예
이 함수는 이렇게 사용하면 안 됩니다. rendering 메서드를 이용하세요.
B) 좋은 예
이 부분은 본래 함수의 의도랑 다르게 쓰인 것 같습니다. https://reactjs.org/docs/testing-recipes.html#rendering 이 문서에서 설명하는 rendering메서드를 사용하는 것은 어떨까요?
텍스트는 딱딱하기 때문에 읽는 사람이 쉽게 오해할 수 있습니다. 평소에 본인이 사용하는 것 보다 훨씬 부드러운 언어를 구사해주세요. 어렵다면 이모티콘이나 움짤로 표정을 담아주면 좋습니다.
A) 아쉬운 예
변수 이름을 분명하게 정의하는 것이 좋다는 건 저도 알고 있습니다.다만 이 부분을 어떻게 바꿔야 할지 잘 모르겠습니다. 아이디어가 있다면 제시해주세요.
B) 좀 더 나은 예
변수 이름을 분명하게 정의하는 것이 좋다는 데에는 저도 공감을 합니다.다만 이 부분은을 어떻게 바꿔야 할지 좋은 아이디어가 떠오르지를 않네요 ㅠ-ㅠ. 좋은 생각이 있으신가요? :)
비난 보다는 제안을 해주세요. 평상시 대화를 할 때 보다 조금 더 조심스러워야 합니다.
A) 아쉬운 예
왜 이렇게 작성을 하셨죠? 코드가 너무 복잡해져서 읽기가 어렵네요.
B) 좀 더 나은 예
useEffect의 clean-up function을 사용하면 코드를 더 간결하게 만들 수 있을 것 같아요!
질문이나 요청을 할 때는 요구하는 내용을 구체적으로 적어주세요. 모호한 질문은 모호한 답변을 낳습니다.
A) 아쉬운 예
이 부분을 잘 모르겠습니다.
B) 좀 더 나은 예
useMemo의 deps에 변수 하나가 빠져있는 것 같은데, 일부러 이렇게 하신 걸까요?