[GIT] 코드리뷰 도입 및 PR 방법

목표

• 능동적이고 즐거운 코드 리뷰 댓글놀이

용어정리

• 리뷰어 : 다른 사람이 작성한 코드를 리뷰하는 사람들
• 리뷰이(요청자) : 본인이 작성한 코드를 다른 사람들에게 리뷰 받는 사람

코드 리뷰 환경

• GitFlow의 형식을 지켜가며, Feature -> Develop -> Staging -> Master 단계로 기능단위(feature) 개발 완료 후 Develop 브랜치로 PR 요청한다

PR 유틸 적용

• Github + Slack 연동 : 연동방법 잘 작성된 다른 블로그를 참조해서 연동해주세요 https://sepiros.tistory.com/37

[Github] github + slack 연동하기

• Github- Slack 주요 명령어 정리
  연결된 Slack 채널에서 아래와 같이 명령어들을 수행할 수 있습니다.

명령어	설명
/github subscribe list	Slack 채널에 연결되어 있는 Repository 리스트 목록 확인
/github signin	Slack 채널에 연결 할 Github 로그인
/github subscribe owner/repository	Slack 채널에 Repository 연결 (구독)
/github unsubscribe owner/repository	Slack 채널에 Repository 연결해제
/github subscribe owner/repository branches commits :all reviews comments	Slack 채널에 연결된 Repository 의 모든 브랜치에 대해서 메시지를 받겠다는 의미
/github subscribe owner/repository issues, pulls, commits:*, releases, deployments, reviews, comments	제가 사용했던 명령어

 

• Github pull_request_template.md 적용하기

### ️✅ 체크리스트 (PR 올리기 전 아래 내용을 확인해 주세요)
- merge 할 Service 브랜치의 위치를 확인해주세요 (master/main❌)
- 리뷰어를 2명이상 선택해주세요
- 승인자는 본인을 선택해주세요
- PR은 승인자 본인이 merge 합니다 (Squash and merge) 
- PR의 라벨(Labels)을 추가해주세요 (PR Status)

### 👉 PR 타입(하나 이상의 PR 타입을 선택해주세요)
- [ ] feature : 신규 기능 개발
- [ ] fix : 버그 수정
- [ ] build : 빌드 관련 수정
- [ ] ci : CI 관련 수정
- [ ] test : 테스트 코드 수정
- [ ] chore : 패키지 매니저 설정할 경우, 코드 수정 없이 설정을 변경

### ✏️ PR 내용
ex) 로그인 시, 구글 소셜 로그인 기능을 추가했습니다

### 💡 PR 리뷰 요구사항 
ex) 집중 리뷰 부분 작성

### 🎸 기타 링크 및 참고사항
ex) 결과물에 대한 스크린샷, GIF, 혹은 라이브 데모가 가능하도록 샘플 API 를 첨부할 수도 있습니다

 

코드리뷰시 지켜야 할 애티튜드

🙌 코드리뷰의 중요성

코드 품질 향상

다른 사람이 면밀히 내 코드를 본다는 생각에, 커밋을 하기 전 한번 더 검토하게 된다. 이는 버그를 줄여줄 뿐만 아니라 더 좋은 품질의 코드를 작성하는데에 도움이 된다. 보다 더 가독성있고, 보다 더 합리적인 코드를 작성하기 위한 고민의 시간을 늘려준다. 또한 ‘왜 이렇게 작성했는지’를 설명하고, ‘왜 이렇게 작성했을까’를 이해하는 과정 속에서 전반적인 프로젝트의 수준을 향상 시킬 수 있다.

버그의 조기 발견

한 사람이 보는 것보다 여러 사람이 보는 것이 버그를 발견하기 유용하다. 내가 생각하지 못했던 것도 누군가는 한 눈에 찾아낼 것이다. 특히나 하나의 fix 작업에 세개의 side effect를 발생시키는 개발자가 조직 내에 있다면.. 중요성은 더 말할 것도 없다.

수준 상향 평등화

다른 사람의 코드를 읽을 기회가 많아지면서, 타인의 코드를 통해 배울 기회를 얻을 수 있다. 또한 코드리뷰를 통해 서로에게 더 좋은 방법을 제시하고 합리적인 방향을 찾아가며 이해 수준이 상향 평등화된다. 단순히 사전에 버그를 발견하거나 문제점을 찾는 목적을 넘어서 전체적인 조직의 역량을 강화할 수 있는 것이다.

개발 표준 준수

정해진 표준 준수를 통해서 많은 사람들의 개발 결과물이 일관된 스타일을 유지할 수 있도록 도와준다. 또한 타인의 코드를 살펴보게 되면서, 같은 작업을 다른 사람이 두번씩 작업하는 일이 없어진다.

책임감 강화

설령 배포된 코드에 문제가 발생했을 때, ‘누가 이 버그를 만들었는지’가 아니라 ‘왜 아무도 이 문제점을 발견하지 못했는지’에 대해 초점이 맞춰진다. 개발 결과물에 대한 책임이 그 코드를 만든 사람 개인이 아닌, 우리 모두에게 있다는 문화를 정착하는 것이다.

인수인계 비용 감수

지속적인 코드리뷰는, 개중 어떤 한 사람이 조직을 떠나게 되었을 때 인수인계 비용을 크게 감소시켜준다. 사실 코드에 대해서는 인수인계를 할 필요도 없는 수준이 되게 되는 것이다.

 

 

 

🚀 올바른 코드리뷰의 방향

✋ 능동적인 참여

사실 상 문화 정착에 있어 가장 중요한 것은 도구보다도 조직원들의 능동적인 참여다. 요청자도, 리뷰어도 모두 자발적인 마음에서 능동적인 태도로 임할 때 가장 빠르게 코드리뷰 문화가 정착될 수 있다.

🛠 효율적인 도구

사실 오프라인으로만 이루어지는 코드리뷰 문화는 절대 고착 될 수 없다. 꾸준히 유지하기에 너무나 큰 비용이 드는 것이다. GitHub이나 GitLab 등의 온라인 리뷰 환경이 구성 될 수 있는 도구를 활용해야한다.

📋 규칙과 강제성

능동성을 갖되, 환경의 강제성이 필요하다. 하고싶은 사람끼리 하고싶을 때만 하는 것이 아니라, 적어도 최소한의 강제성을 가지고 잘 정리된 환경 속에서 규칙을 준수해야하는 것이다.

 

 

 

✍️  리뷰어의 자세

어쨌든 다른 사람에게 의견을 준다는 것은 논쟁으로 이어질 수 있다. 최대한 둥글고 둥글게 진행되어야한다. 또한 피드백을 주려는 자세 외에도, 상대방의 좋은 점을 배우려는 자세가 필요하다. 후임에게서도, 주니어에게서도 나는 생각치 못했던 점들을 배울 수 있다. 단순히 리뷰를 해주는 것에 그치지 않고 내 것으로 만들기 위한 노력이 필요하다.

🎓 고수준으로 시작, 저수준으로 내려가라

• 리뷰 라운드에서 많은 의견을 남길 수록, 요청자가 당황할 위험이 커진다.(내가 잘… 못하나?)
• 하나의 라운드에 20~50개 정도의 의견은 위험하다. 10개를 초과하지 않는다.
• 초기 라운드에서는 고수준 피드백으로 제한해라
  • 버그, 장애, 성능, 보안 등
• 고수준의 피드백이 처리된 후 코드리뷰가 적응되었을 경우, 저수준 이슈를 처리한다.
  • 설계 개선 (디자인패턴 등등..)
  • 변수명 변경, 주석을 명확하게 작성 요청 등등 (코드컨벤션, 코드스멜 잡기)
  • 저수준은 요청자 스스로 알잘딱깔센 하게 하자

📍 피드백 방법

• 절대 “너” 라고 하지 마라 (너는, 왜, 맨날… 부정적인 어투 사용금지)
• 건설적인 피드백을 해라
• 요청자가 잘 작성한 코드에 대해서도 승인(Approve) 시 칭찬을 많이 해라👍
• 피드백은 명령이 아니라, 요청으로 표현해라 (이런 방식으로 변경 해주세요(X), 이런 방식도 있는데 이렇게 고려해 보는건 어떻게 생각하세요?)

👍  리뷰를 위한 리뷰를 하지 마세요. 피드백 할 게 없으면 칭찬해 주세요.

리뷰할 부분이 없는데 흔적을 남기기 위해 리뷰할 경우 서로 어색해지는 경우도 발생합니다. 예를 들어, 깔끔하게 짜인 코드에 다른 디자인 패턴을 권유하는 것입니다. 리뷰이 입장에서는 잘 짜인 코드임에도 불구하고 안 좋은 코드라 여기고 잘못된 방향으로 아키텍처가 설계될 수 있습니다. 또는 앞뒤의 연관 관계없이 “코드가 복잡해 보입니다.”, “코드가 읽기 어렵습니다.”와 같은 리뷰는 서로 감성소비만 심해집니다. 따라서 리뷰할 부분이 없으면 칭찬 피드백을 주는 것도 한 가지 방법입니다.

아래는 칭찬 피드백에 대한 예시입니다.

• 코드량이 적당해서 읽기 편하네요.
• 많은 고민이 코드에서 엿보이네요.
• 성능에 아주 유리한 코드라고 생각되네요.
• 전에 코드보다 훨씬 좋아진 거 같네요.
• 예외 처리가 꽤 꼼꼼해서 좋네요.
• 함수, 변수명이 직관적이어서 좋네요.

‘칭찬은 고래도 춤추게 한다.’라는 속담도 있듯이 리뷰할 때 좋은 코드, 깔끔한 코드를 보고 칭찬한다면, 서로 시너지 효과를 증대시킬 수 있습니다.

 

🥇 리뷰는 즉시 시작

• Slack을 통한 리뷰를 요청 받았을 경우, 코드 리뷰를 높은 우선순위로 시작해라
  • 요청자는 리뷰 종료될 때까지 대기 해야함
• 리뷰를 바로 시작하면 선순환 됨
  • 코드를 읽고 피드백을 줄 때는 시간을 가지고 진행해도 되지만, 요청을 인지 했다면 시작은 바로 해라
• 리뷰 라운드의 최대 시간은 하루(24시간)
  • 우선순위 높은 업무로 24시간 내에 불가하면, 다른 리뷰어 지정한다
  • 하루가 넘길 동안, 리뷰어가 코멘트 하지 않을 경우, 저자가 직접 merge 한다
  • 반드시 리뷰어로 부터 하나 이상의 승인(Approve)이 있어야 Merge 가능하다 (요청자, 리뷰어 둘다 Merge 가능)
  • 직접 Merge 하여 문제가 생겼어도, 요청자의 잘못이 아닌 팀 전체가 간과한 잘못임을 인지하자

✏️ 답을 알려주기보다는 스스로 고민하고 개선 방법을 선택할 수 있게 해주세요

// 안 좋은 리뷰
“arr.filter(item => item % 2 === 0);으로 사용하세요." 

// 좋은 리뷰
“자바스크립트에는 배열에는 다양한 내장 메서드들이 존재합니다. 
그중 filter 메서드를 활용해 보세요.
이 메서드를 활용하면 코드량을 줄일 수 있습니다.”

답을 알려주기보단 키워드(filter)를 알려줌으로써 어떤 식으로 검색해야 하는지 방법을 제시하고 있는 피드백입니다. 리뷰이 입장에서는 동작 방식을 찾아보고 스스로 학습할 기회가 생기고 비슷한 코드를 리팩토링할 수도 있습니다.

 

 

 

🤝 리뷰이(요청자)의 자세

🤐 교착상태를 적극적으로 처리해라

• 코드 리뷰의 최악의 결과는 교착상태이다 (Stalemate)
  • 요청자는 승인(Approve)을 받지 못했기에 Merge 불가 상태
  • 리뷰어는 요청자의 코드의 어떠한 면이 맘에 들지 않아 요청자가 수정할때 까지 대기 상태(WAIT)
• 교착상태에 빠진 PR은 리뷰어 전체가 만나서 얘기하라(대면, 화상X)

🤞 리뷰어를 배려하라

• 코드 컨벤션을 잘 지켜라. 깔끔하고 잘 동작하는 코드를 만들기 위해서 정해 놓은 규칙인데 이를 무시하거나 인지하지 못한 채로 커밋을 작성하게 되면 이에 대한 리뷰가 먼저 들어올 것이다. 이는 불필요한 코멘트이고 시간 낭비가 될 수 있으니 지양하는 것이 좋다.
• 리뷰 가이드라인을 잘 작성해라. 모든 코드 변경사항에는 의도가 필요하다. 의도치 않게 변경된 부분이 있다면 되돌려 놓아야 하고, 줄바꿈과 같이 아주 단순한 변경사항이라도 그 부분을 리뷰어가 볼 필요가 없다면 “Just line change” 와 같은 코멘트를 달아 명시하여 리뷰 시간을 줄여줄 수 있을 것이다. 또는 사용된 라이브러리 업데이트가 포함되었다면 해당 라이브러리의 릴리즈 노트 링크나 스크린샷을 첨부하는 것도 좋은 방법이다.

🤞 반드시 테스트하라

• 베이스 브랜치에 포함되기 위한 코드는 모두 정상적으로 동작해야 한다. 너무도 당연한 얘기지만 리뷰어가 내 코드를 직접 돌려보고 테스트하도록 만드는 것보다 내가 직접 돌려본 결과 이상이 없다는 것을 증명하는 것이 더 빠르고 효율적인 방법이다.
• 만약 직접 실행을 통해 확인이 필요한 기능이라도 스스로 검증한 내용은 첨부하되, 어떻게 테스트하면 될지에 대한 가이드라인을 리뷰어에게 주는 것 또한 하나의 방법일 것이다.

 

 

우리들의 코드리뷰 도입

🤙 약속

1. JIRA 이슈 생성
2. feature/{Service}/{JIRA 이슈번호} 브랜치에서 작업한 코드들을 commit&push한다
3. feature/{Service}/{JIRA 이슈번호} > Service 브랜치로 merge하기 위한 pull-request를 생성한다
4. 구성원들이 해당 PR을 리뷰한다 (Comment / Approve / Request changes)
5. 1개 이상의 approving이 되면 개발자 혹은 assignees가 해당 브랜치를 Service 브랜치로 병합한다
6. PR Labels 를 활용하자 : PR의 상태값을 나타내서 해야할 리뷰인지 구분짓기 위함
7. 라벨 생성
   • 👯‍♂️ 0. 논의필요/해당 PR은 대면해서 이슈 해결 필요합니다/#B60205
   • 🚨 0. 긴급/최우선 처리되어야 할 PR/#B60205
   • 🙅‍♂️ 1. 대기중/리뷰이가 PR은 등록했지만, 아직 리뷰하지 말아주세요/#D93F0B
   • 🙇‍♀️ 2. 리뷰필요/리뷰가 필요합니다/#FBCA04
   • 🚀 3. 리뷰중/리뷰중인 PR/#0E8A16
   • 🤩 4. 병합대기/리뷰어가 Approve 처리한 PR 이며, 병합 대기중입니다/#0052CC
   • ✨ 5. 리뷰완료/PR 리뷰 완료 상태입니다/#5319E7
   • 🎉 Staging Deploy/Staging 배포/#54DCDC
   • 🎉 Prod Deploy/Prod 운영 배포/#F155AD

👍 리뷰하기

pull-request를 리뷰하는 데는 3가지 상태로 리뷰할 수 있다

• Comment : 승인하거나 변경 요청 없이, 일반적인 피드백을 주는 것이다
• Approve : 브랜치 병합을 허용한다. 보통 👍 표시와 함께 approve해준다 (따봉 표시를 받으면 기분이 좋다)
• Request changes : 병합전에 해결해야 하는 사항을 피드백해준다 (예를들어, xxx 부분의 side effect 이 예상되니 고려해서 소스코드 수정해주세요)
• 수정사항 별로 피드백하기
  위 과정처럼 pull-request에 대해 한번에 리뷰할 수 있지만, 수정사항 별로 개별 의견을 남길 수도 있다.
  각 수정사항 별로 리뷰하고, 요약설명과 함께 Approve하거나 Request changes할 수 있다.
  Comment나 Approve는 피드백을 주는 것이 전부지만, Request changes는 파일/라인 별로 수정사항을 피드백 할 수 있다.

 

🥳 리뷰받기 & 병합하기

• 리뷰받은 내용에 대해 코멘트를 달아주거나, 소스코드를 수정해서 commit&push 하면 자동으로 해당 PR에 이력으로 쌓인다.
• Approve 최소 인원 설정하여, 충족되면 Merge pull request 버튼이 활성화 된다. 드롭다운으로 다음을 수행할 수 있다.
  
  • Merge pull request: feature의 모든 커밋이 base 브랜치로 병합되어 커밋된다.
  • Squash and merge: feature의 모든 커밋이 하나의 커밋으로 병합된다.
  • Rebase and merge: 모든 커밋이 merge 커밋 없이 개별적으로 base 브랜치에 추가 된다.
• 필요에 따라 작업을 완료한 feature 브랜치를 삭제한다. Repo를 깔끔하게 유지된다.
  • 설정방법 : Github > repo > settings 에서 Genernal 에서 'Automatcially delete head branches' 옵션 제공
    
    

참조

1. https://blog.cowkite.com/blog/2003062358/ https://tech.kakao.com/2022/03/17/2022-newkrew-onboarding-codereview/
2. https://phillip5094.tistory.com/80
3. https://velog.io/@ye-ji/Git-PR-ISSUE-템플릿-등록하는-방법