코드리뷰를 위한 지침 '코드리뷰 피라미드'

코드리뷰 피라미드

관련글만 보시려면 아래로 쭉 스크롤해주세용

⭐서론(겸 잡담)

아래 페이지에서 코드 리뷰 피라미드에 대한 글을 보았다.
관련글 : https://www.morling.dev/blog/the-code-review-pyramid/
요즘 회사에서 개발 문화를 향상시키기 위해서 이런 저런 노력들을 하고 있는데, 그 중에 코드리뷰가 있다.
내가 코드리뷰에 어떤 노력들을 쏟고 있는지와 함께 코드리뷰 피라미드 글을 해석해보았다.

⭐코드리뷰의 궁극적인 목표(+ 내 생각)

1. 목표

• 유지보수 효율 업(코드의 가독성 및 비즈니스 이해도 증가 등)
• 사전 오류 검출
• 역량 강화(팀원의 코드 수준이 전체적으로 강화된다.)

2. 장점

• 코드리뷰를 받는다는 생각에 코드를 한번 더 보고 조금 더 심사숙고해서 짜게 된다.
• 내가 보지 못한 오류에 대해서 조기 발견이 가능하다.
• 리뷰어도 코드리뷰를 하기 위해 실력이 향상된다.
  • 부담감과 책임감이 있으니깐 아무래도 좀 더 공부를 하게 되는 것 같다.
• 여러 사람의 아이디어가 모여서 더 나은 코드가 완성된다.
  • ex) 여기서는 이런 방법을 활용했는데 좋았어요. 등등 내가 몰랐던 부분을 알게 될 수 있다.
• TMI로 비즈니스에 대한 이해도 가능하다
  • 리뷰하면서 이 코드가 쓰이는 비즈니스나 히스토리에 대한 언급이 자연스레 있을텐데, 그러면서 서로 이해도가 올라갈듯

3. 단점

• 시간을 많이 쓴다.
  • 리뷰어는 남이 짠 코드를 보고 이해하고,피드백하는데 아무래도 시간이 걸린다.
  • 리뷰를 받는 입장에서는 잘 돌아가는 소스여도 리뷰를 받고 또 수정해야 할 수도 있으니깐?
• 자칫 잘못하면 감정소모가 된다.
  • 대부분의 코드리뷰에 대한 글,영상에서 이 부분에 대한 언급이 정말 많았다. 그래서 코드리뷰를 할 때 규칙을 정해서 많이 하더라.

---

⭐코드리뷰를 습관화 하기 위해 내가 회사에서 하고있는것들

0. 나를 믿고 고집부리기

• 팀원이 하게 만들려면 일단 어느정도 고집을 가지고 끌고 나가야 한다고 느꼈다. 안그러면 흐지부지 되어버리고 만다.

1. GITLAP의 MR을 통한 코드리뷰와 MR 템플릿 (깃의 경우는 PR)

• MR시 생성되는 chnage탭을 통해 코드리뷰를 하고 있다.

• MR 템플릿은 SK 데보션의 코드리뷰 문화 포스팅에서 템플릿을 고대로 가져와서 약간 변경해서 사용했다.
  • 쓰다보니 약간 가독성이 떨어지는것같다는 지적을 받았다. (너무 요란하다고 함 😥😥)

코드 리뷰 문화를 리뷰해 봐요 (코드 리뷰 프로세스 개선 ＆ PR Reminder Bot 개발 이야기)

MR 템플릿

2. Gitlab의 이슈보드와 라벨을 활용

• 
• 리뷰어 뿐만 아니라 다른 팀원들도 코드리뷰에 적극적으로 참여했으면 하는 마음에 이슈보드의 한 구역을 코드리뷰로 지정했다.
• 우리회사는 일단 웬만한 외부망은 전부다 막혀있고, 슬랙같은 메신저 라이센스도 없기 때문에 차선책으로 ... (😥)

3. 코드리뷰 요청 가이드 및 잔소리

• 혼자 이것저것 정리해서 만든것이기 때문에 팀원들은 아직 익숙하지 않다. 그래서 코드리뷰 요청 가이드를 만들고 요청이 익숙해질 수 있게 잔소리(싫지않게😥)를 마구 하고 있다.

  

  코드리뷰 요청 가이드의 일부분

  

4. 이슈브리핑 메일

• 

  이메일로 매일매일 발송하고 있습니다.

• 깃랩 API를 활용해서 (2)에서 지정했던 이슈들을 한번 더 확인할 수 있게 메일로 쏴주게 했다.(슬랙 라이센스좀 사줘)

5. 공부하기

• 유튜브, 온라인강의, 독서등을 통해 공부를 한다. 점심시간이나 이동시간을 주로 활용한다.

독서와는 좀 거리가 있어서 읽는데 오래 걸리는 중

• 요즘 읽고있는 책은 좋은 코드, 나쁜 코드로 실제로 책에서 읽은걸 바탕으로 코드리뷰를 하기도 했다.

6. 코드 스타일가이드

• 회사에 표준 코딩 가이드가 없기 때문에 일단 유~명한 구글의 스타일 가이드를 공부했다. 우리 회사 서비스에 이걸 적용하기에는 아직 짬이 덜차서😥 벅찬것 같다. 좀더 우리 회사에 맞게 수정도 필요하기도 하고 (?)
• 사내 표준 스타일 가이드가 없기 때문에 우선 코딩 스타일에 대한건 왠만하면 코드리뷰시 언급하지 않는다.
• 구글의 JAVA 스타일 가이드 번역 (https://janggiraffe.tistory.com/405)

더 나은 개발 문화를 위해 #1 코딩 스타일 가이드(Google Java Style Guide 번역)

⭐코드리뷰 피라미드 번역 + 내 생각

• 다시한번 얘기하지만 다음 페이지의 글을 해석한것 + 내 생각을 썼다. (https://www.morling.dev/blog/the-code-review-pyramid/)

The Code Review Pyramid

 

• 개인 의견은 📢 아이콘으로 표기하겠음.
• 코드리뷰시 어떤 측면에 초첨을 맞춰서 코드리뷰를 해야 하는지에 대한 지침을 제공하기 위해 본문의 개발자분이 공유한 자료이고, 이를 '코드 리뷰 피라미드'라고 부르기로 했답니다. 이 그림을 보고 코드 리뷰를 할 때 어떤 부분이 중요한지, 어떤 부분은 자동화 할 수 있는지 고려할 수 있을 것 같습니다.

오늘의 본문 'The Code Review Pyramid'

[자동화 할 부분 : 1. Code Style ~  2.Tests]

1. Code Style

1. 프로젝트의 서식 스타일이 적용됐나?
   • 📢 이런식으로 자동화 할 수 있을듯 하다 https://janggiraffe.tistory.com/408(구글 자바 포매터 적용해보기)
   • 📢 또 다른 방법으로는 spotlessApply라는게 있는듯한데 이거는 공부 예정

   • 구글 자바 포매터 적용해보기(for eclipse)

2. 합의된 명명 규칙을 준수하는가?
3. DRY원칙을 따르는가?
   • 📢 Don't Repeat Yourself의 약자로 소프트웨어 개발 원칙중 하나로, 코드의 중복을 피하고 재사용성을 높혀 코드의 효율성을 높히고, 유지보수를 간단하게 만들려는 목적을 가지고 있음.
4. 코드가 충분히 가독성이 있는가? (메서드 길이 등..)

2. Tests

1. 모든 테스트가 통과했나?
2. 새로운 기능은 합리적으로 테스트되었는가?
3. 극단적인 경우들이 테스트 됐나? 📢 오류케이스가 인입되서 분석해보면 사용자는 정말 다양한 방법으로 서비스를 사용했엇다. 정말 극단적인 것들도 테스트가 필요할듯 하다.
4. 가능한 경우 단위 테스트를 사용하고 필요한 경우 통합테스트를 했나?
5. 비기능 요구사항(NFRs)을 위한 테스트가 있는가?(ex. 성능 테스트)
   • 📢 (반댓말은 기능적 요구사항 : 시스템이 뭘 해야 하는가?)
   • 📢 어떻게 시스템이 동작해야하는가로 성능,보안,확장성 등등을 의미

• 📢 내가 제일 약한부분으로 앞으로 공부해나가야 하는 부분이다.

[코드리뷰에 집중해야 할 부분:3. Documentation ~ 5. API Semantics ]

3. Documentation

1. 새로운 기능이 합리적으로 문서화되었는가?
2. 해당 종류의 문서가 다루어져 있는가: README, API 문서, 사용자 안내서, 참조 문서 등?
3. 문서가 이해하기 쉽고 중요한 오타나 문법 오류가 없는가?

• 📢 문서화는 서비스를 운영하는데 있어 진~ 짜 중요하다고 생각하지만 문서 업데이트는 생각만큼 쉽지 않다. 어떻게 하면 잘 할 수 있으려나..

4. Implementation Semantics

1. 요구사항을 만족하는가?
2. 논리적으로 옳은가?
3. 불필요한 복잡성이 없는가?
4. 강건한가? (동시성 문제 없음, 적절한 오류 처리 등)
   • 📢 동시성 문제 : 서비스에서 여러 스레드가 하나의 자원을 동시에 접근하려 할때 발생하는 문제.(데드락 등등이 있을 수 있는지 체크)
5. 성능이 우수한가?
6. 보안성이 확보되었는가?(ex. sql 인젝션 방지 등)
7. 관찰 가능한가? (지표,로깅,추적 등)  📢 경험 상 운영 적용 후에 예상치 못한 방법들의 오류가 발생했었다..
8. 새로 추가된 의존성이 그 가치를 제공하는가? 라이선스는 허용 가능한가?

5. API Semantics

1. 가능한 작게, 필요한 만큼 크게
   • 📢 API as small as possible : API는 필요한 기능을 충족시키기 위한 최소한의 요소로 유지되어야 함.
   • 📢 as large as needed : 그러나 API는 사용자의 요구사항을 충족시키기 위해 충분히 크게 설계되어야 함.
   • 📢 지나치게 크게 만들면 복잡성이 증가하고 이해하기 어려워지며 지나치게 작게 만들면 사용자가 필요한 작업을 수행하는데 부적합할 수 있어 적절하게 만들라는 의미이고, 적절한건 서비스마다 다를듯 하다.
2. 한가지 일을 하는 방법이 하나뿐이며, 다양한 방법이 없는가?
   • 📢 API 메서드가 여러가지 방법으로 제공되어 혼란을 야기하는지 물어보는 듯
3. 일관적인가? 놀람 최소 원칙(principle of least surprises)을 따르는가?
   • 📢 개발자가 예상하는 동작을 제공하는지를 체크해야 하는듯.
4. API와 내부 구현이 깨끗하게 분리되어 있으며, 내부 구현이 API로 누설되지 않는가?
5. 사용자에게 노출되는 부분에서 파손을 일으키는 부분은 없나?
6. 새로운 API가 일반적으로 유용하며 지나치게 구체적이지 않은가?
   • 📢 API가 특정 상황에 제한되게 만들어지진 않았는지 체크하는 부분으로, 일반적으로 사용 가능하게 짜라는 듯