Git - Pull Request(PR) 매너 및 주의점 !! ***필독***

 

취업 준비 및 프로젝트를 진행하며 Git을 적극적으로 활용했습니다. 본인의 경우 초반에는 프론트앤드 지인(친구)과 함께 프로젝트를 진행하다 보니 커뮤니케이션이 너무 원활했습니다. 그렇게 진행하다 보니 협업에서는? 어떻게? 진행할까 궁금해서, 인프런에서 프로젝트 및 스터디 게시글을 올려 백앤드, 프론트앤드, 디자이너를 영입해서 진행했습니다. 프로젝트 팀장 및 백앤드를 담당하면서, 팀원과 어떻게 하면 커뮤니케이션 및 Git을 잘할 수 있을까 해서 이 글을 작성해 봤습니다.

그래서 이 글은 프로젝트 협업 관련해서 Git Pull Request를 보낼 때 매너 및 주의점에 대해 알아보겠습니다.

1. 코드 리뷰란?

코드 리뷰는 동료가 작성한 코드를 점검하고 피드백을 교환하는 과정입니다. 피드백을 교환화는 과정에서 잠재적인 오류나 버그를 발견하여 개선하고, 코드의 가독성 및 유지보수성을 높이기 위해 수행됩니다. 이러한 피드백을 바탕으로 코드 작성자 및 팀 전체의 코드 퀄리티를 향상 시킬 수 있습니다. 즉, 단순히 오류나 버그를 찾는 것뿐만 아니라, 코드의 구조, 설계, 가독성, 유지보수성, 성능 등을 평가하고 피드백을 제공하여 팀 전체의 코드 퀄리티를 향상 시키는 역할을 합니다. 또한, 코드 리뷰는 팀 내의 지식 공유와 개발자 간의 의사소통을 촉진하는 역할도 합니다.

2. 코드 리뷰의 기본 원칙

• 모든 코드는 코드 리뷰를 마친 후에 머지(merge)를 해야 합니다.
• 혼자가 아닌 함께 하는 일이기에 우리 모두는 코드 리뷰에 책임감을 갖습니다.
• 1명의 필수 리뷰어가 PR을 승인해야 머지할 수 있습니다.
• 인프라 작업이나 릴리즈 PR은 리뷰를 생략할 수 있습니다.
  • 단, review skip 레이블을 붙여서 스프린트 리뷰 시간에 일괄 검토합니다.
• PR을 받았으면 하루 안에 답변을 줘야 합니다. [답변이 없으면 재촉하세요.]
• 리뷰하기에 양이 많다면, 오프라인 리뷰를 요청하시길 바랍니다.
• 특별히 신경을 써야 할 리뷰 포인트는 코멘트를 남겨주시면 좋습니다.
• 자신의 취향을 주장하기보다는 사실과 데이터를 가지고 의견을 교환해 주는 게 좋습니다.

3. 코드 리뷰를 요청할 때 지켜야 할 매너

코드 리뷰는 PR(Pull Request)을 만드는 것에서 시작합니다.

충분한 정보를 제공하는 PR은 리뷰어가 더 정확하고 신속한 리뷰를 할 수 있도록 돕습니다.

3.1 변경한 내용에 대한 충분한 정보를 제공하기

PR의 목적을 잘 전달하기 위해서는 충분한 정보를 동료에게 제공해야 합니다.

코드 리뷰를 하는 동료는 내가 아는 것들을 모른다고 가정하고 아래의 내용을 잘 따라주시면 감사하겠습니다.

커밋 메시지를 충실히 작성하기

커밋 메시지의 제목만 봐도 어떤 작업을 했는지 유추할 수 있어야 합니다.

제목은 글자수 제한이 있기 때문에 어떤 작업을 했는지 설명하기 어려울 때가 있습니다.

이럴 때는 제목에 간략히 작업 내용을 요약해서 적고, 상세한 내용은 본문에 작성을 해주시면 됩니다.

PR 본문을 충실히 작성하기

PR의 본문에도 PR을 작성한 계기, 수정 방향, 관련 이슈 등을 충실히 적어야 합니다.

아래의 PR 템플릿에 맞춰서 본문을 작성해 주세요.

<!--
  제목은 `[(키워드)] (작업한 내용) - (PR 순서)` 로 작성바랍니다.
-->

## 설명

### 작업 티켓 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 레이블을 추가했나요?

3.2 작은 PR 만들기

왜 작은 PR이어야 할까?

• 변경한 코드의 양이 적을수록 리뷰를 하기가 더 쉽습니다.
• 변경한 코드가 적을수록 버그가 있을 가능성이 낮습니다.
• 동료가 더 짧은 시간 동안에, 더 집중해서 리뷰를 할 수 있습니다.
• 코드 리뷰에서 받은 피드백을 적용하기가 더 쉽습니다.

작은 PR의 조건

• 하나의 PR에는 하나의 작업만 담겨 있어야 합니다. 하나의 PR에 여러 작업이 포함되면 리뷰를 하기가 어려워집니다.
• 주석, 커밋, PR 본문 등, 리뷰를 하는 데에 필요한 모든 정보를 충실히 제공해야 합니다.
• 새로운 함수나 API를 추가할 때는, 해당 API 또는 함수를 사용하는 코드도 PR에 포함시켜야 합니다.

4. 코드 리뷰를 할 때 주의할 점

4.1 코드 리뷰를 할 때 보아야 할 것

설계

사내 애플리케이션 설계 원칙과 일반적인 프로그래밍 설계 지침에 맞춰서 코드가 작성되어 있는지 확인합니다.

비즈니스 로직을 몰라도 리뷰를 할 수 있는 사항들

팀원 모두가 같은 도메인에서 작업하지 않기 때문에 도메인의 비즈니스 로직에 대해서는 리뷰를 하기가 어렵습니다. 하지만 코드 그 자체를 리뷰할 수는 있습니다. 이 섹션은 코드 그 자체를 리뷰할 때 중요하게 보아야 할 부분을 설명합니다.

유지보수성

• 중요한 값을 하드 코딩을 하고 있지는 않은지?
• 주석이 코드가 하는 일이 아닌, 코드에 담긴 의도를 설명하고 있는지?
• 코드를 쉽게 이해할 수 있는지?
  • 코드만 봐도 작성자의 의도를 이해할 수 있는지?
  • 비즈니스 로직을 잘 설명하고 있는지 확인합니다.
  • 올바른 변수명, 함수명, 클래스명을 사용했는지 확인합니다.
  • 내가 이해할 수 없는 코드는 다른 동료도 이해할 수 없습니다. 그러니 작성자에게 설명을 요구하거나 개선을 부탁합니다.
• 설정이 용이한지? 코드의 특정 부분을 설정 파일(. env) 파일을 사용해서 설정을 변경하게 만들 수 있는지 확인합니다.
• 스타일 가이드에 맞게 작성되어 있는지?
• 하나의 함수가 10 라인을 넘어간다면 너무 많은 관심사를 갖고 있는 건 아닌지 유심히 봐줍니다.

재사용성

• 중복된 코드는 없는지?
• 두 번 이상 반복해서 등장하는 코드는 함수나 클래스로 분리하는 걸 검토합니다.

안정성

• 예외 처리를 제대로 하고 있는지?
• 자원을 효율적으로 반환하고 있는지? 예) useEffect의 clean up function에서 제대로 초기화를 했는지

확장성

• 새로운 기능을 추가하기 쉽게 작성되어 있는지?
• 함수나 클래스가 두 개 이상의 관심사를 갖고 있지는 않은지 확인합니다.

테스트

• 모든 코드는 테스트하기가 쉬워야 합니다.
  • 큰 함수를 작은 단위로 나눕니다.
  • 클래스 간에 모킹 하기 쉬운 인터페이스로 연결되어 있는지 확인합니다.
• 테스트가 작성되어 있는지 반드시 확인을 합니다.

4.2 리뷰 중인 코드 보는 방법

• 다음과 같은 단계로 리뷰를 합니다.
  1. 전체 코드를 훑어보기
     • 코드 전체에서 일어나는 변경을 가볍게 훑어봅니다.
     • 의미가 없는 변경점이나 변경이 일어나서는 안 되는 곳이 있는지 확인합니다.
  2. PR의 주안점 확인
     • 리뷰를 받는 사람이 제시한 주요 변경사항을 확인합니다.
     • 주안점이 제시되어 있지 않다면 PR 작성자에게 설명을 요구합니다.
     • 구조를 변경한 PR은 빠르게 피드백을 전달하고 동료에게 전파를 해야 합니다.
  3. 놓치는 파일이 없도록 모든 부분을 꼼꼼하게 확인합니다.
• 잘한 점이 있다면 칭찬합니다!
• 리뷰를 해야 할 코드의 양이 너무 많다면? 오프라인으로 리뷰를 요청합니다.

4.3 언제 코드 리뷰를 하면 좋을까?

• 코드 리뷰는 요청을 받는 즉시 하는 걸 원칙으로 합니다.
• PR을 받았으면 하루 안에는 답변을 주세요. 상대방의 답변이 없으면 재촉합니다.
• 자리에 앉아서 다른 업무를 하기 전에 코드 리뷰를 하는 습관을 가집니다.
  • 출근 직후
  • 점심 먹고 자리에 앉았을 때
  • 회의나 화장실에 갔다가 돌아와서 업무를 시작하기 전에

5. 커뮤니케이션 매너

코드 리뷰에 의견을 작성할 때는 커뮤니케이션에 특히 신경을 써야 합니다.

대부분의 리뷰를 텍스트로 진행하므로 오해가 생기기 쉽습니다. 정확한 사실을 부드럽게 전달하도록 노력해야 합니다.

5.1 동료의 능력을 존중해 주세요.

동료가 작성한 코드는 나와 다른 관점에서 작성한 것일 수 있습니다.

리뷰를 하기 전에 작성한 의도를 먼저 물어봐주시고 동료의 의도를 이해하려고 노력해야 합니다.

5.2 코드는 팀의 자산임을 기억해 주세요.

우리는 팀으로 일을 합니다. 우리가 작성하는 모든 코드는 내가 아닌 팀의 자산입니다.

자신의 취향을 주장하기보다는 협업을 하기에 좋은 코드를 찾는 데에 힘을 모아야 합니다.

5.3 객관적인 자료를 제시하세요.

권장 사례(Best Practice)를 설명한 문서나, 공식 문서 등을 활용해서 의견에 근거를 제시합니다.

A) 좋지 않은 예
이 함수는 이렇게 사용하면 안 됩니다. rendering 메서드를 이용하세요.

B) 좋은 예

이 부분은 본래 함수의 의도랑 다르게 쓰인 것 같습니다.
<https://reactjs.org/docs/testing-recipes.html#rendering>
이 문서에서 설명하는 rendering메서드를 사용하는 것은 어떨까요?

5.4 텍스트는 작성자의 의도를 읽기 어렵다는 걸 잊지 마세요.

텍스트는 딱딱하기 때문에 읽는 사람이 쉽게 오해할 수 있습니다.

평소에 본인이 사용하는 것보다 훨씬 부드러운 언어를 구사해 주세요.

어렵다면 이모티콘이나 움짤로 표정을 담아주면 좋습니다.

A) 아쉬운 예

변수 이름을 분명하게 정의하는 것이 좋다는 건 저도 알고 있습니다.
다만 이 부분을 어떻게 바꿔야 할지 잘 모르겠습니다. 아이디어가 있다면 제시해주세요.

B) 좀 더 나은 예

변수 이름을 분명하게 정의하는 것이 좋다는 데에는 저도 공감을 합니다.
다만 이 부분은을 어떻게 바꿔야 할지 좋은 아이디어가 떠오르지를 않네요 ㅠ-ㅠ. 좋은 생각이 있으신가요? :)

5.5 상대를 비난하지 않도록 주의해 주세요.

비난보다는 제안을 해주세요. 평상시 대화를 할 때 보다 조금 더 조심스러워야 합니다.

A) 아쉬운 예

왜 이렇게 작성을 하셨죠? 코드가 너무 복잡해져서 읽기가 어렵네요.

B) 좀 더 나은 예

useEffect의 clean-up function을 사용하면 코드를 더 간결하게 만들 수 있을 것 같아요!

5.6 정확하게 요구를 해주세요.

질문이나 요청을 할 때는 요구하는 내용을 구체적으로 적어주세요. 모호한 질문은 모호한 답변을 낳습니다.

A) 아쉬운 예

이 부분을 잘 모르겠습니다.

B) 좀 더 나은 예

useMemo의 deps에 변수 하나가 빠져있는 것 같은데, 일부러 이렇게 하신 걸까요?

6. REF [우테코]

• Code Review Checklist
• 구글 코드 리뷰 가이드
• 좋은 git 커밋 메시지를 작성하기 위한 7가지 약속
• Code Health: Respectful Reviews == Useful Reviews