코드리뷰

코드 리뷰 전

코드에 대한 것은 리팩토링, 클린코드, 클린아키텍처 등 개인이 공부해야 함
- 나중은 오지 않는다
- 빠르게 만드는 것은 잘 만드는 것이다
코드 리뷰는 작성자를 비판하는 것이 아님을 인지
- 작성자도 리뷰어도 생각해야 함

좋은 코드리뷰?

개인이 생각하는 코드리뷰란 무엇인가? → 코드리뷰할 때 생각하는 것
개인이 받고싶은 코드리뷰란 무엇인가? → 코드리뷰 받을 때 받고싶은 것
코드리뷰가 도입되었으면 한다고 말할때 생각하는 이상향?
신입들이 받고 싶은 경력자의 코드리뷰란?
신입들이 받고 싶은 동료의 코드리뷰란?
경력자가 받고싶은 코드리뷰란?
내가 받았던 좋은 코드리뷰의 예시?

References

글로벌기업은 코드 리뷰를 어떻게 할까요? - SAMSUNG SDS

Google은 코드 리뷰 개발자 가이드를 통해 주요 원칙을 정함
Google은 코드 리뷰가 요청되고나서 1영업일 이내에 반드시 완료되어야 하는 것을 원칙으로 함 → 코드 리뷰가 늦어지면 팀 전체의 개발 일정이 문제가 된다고 판단
Google의 코드 리뷰는 작은 변경은 1시간 이내, 큰 변경은 5시간 이래에 검토가 완료됨. (평균 4시간)
Google의 코드 리뷰의 90%는 10개 미만의 파일로 구성되며, 리뷰의 75%는 리뷰어가 한명뿐.
- 주요 부분을 작게 나누어 리뷰할 수 있도록 변경 요구사항(Change List; CL)을 여러 개의 CL로 분할한 다음 적절한 순서로 살펴보도록 권장함
Microsoft의 개발자라면 매일 코드 리뷰를 해야함
Microsoft의 전체 개발자중 75%가 매일 코드 리뷰를 하고 있다고 함

코드리뷰의 진짜 목적은 따로있다 - LOGISPOT

"코드리뷰란, 한 명 또는 여러 명의 개발자가 본인이 만들지 않은 코드의 내용을 점검(examining)하고, 피드백을 주는 과정을 말합니다."
코드에 대한 책임이 모두에게 있다는 문화를 만드는 목적
코드 리뷰의 장점
- 버그의 조기 발견
- 개발 표준 준수
- 중복 코드 방지 및 모듈 재사용성 증대
- 잘 만들어진 코드를 보면서 배울 기회, 이해 수준을 상향 평준화. → 조직의 역량을 강화하는 중요한 역할
코드 컴플릿(Code Complete)의 저자 스티브 맥코넬은 책에서 다음과 같은 실험결과를 통해 코드리뷰의 중요성을 강조함

소프트웨어를 유지보수하는 조직에서 코드 한 줄을 변경한다고 했을 때, 코드리뷰가 도입되기 전에는 그러한 변경의 55% 정도가 문제를 일으켰다. 그러나 리뷰 과정이 도입된 이후에는 그러한 변경의 2% 정도에서만 문제가 발생했다.

— Steve McConnell
author of Code Complete
코드 리뷰의 핵심은 '개발 결과물에 대한 책임이 그 코드를 만든 사람 개인에게 있지 않고, 우리 모두에세 있다는 문화를 정작하는 것'
코드 리뷰를 수행하는 방식은 매우 다양하며 조직마다 다름. 일반적으로 두 가지 종류로 분류할 수 있음
- 정형화되고 공식적인 코드 리뷰(formal code review)
  - 좀 더 전통적인 방식으로, 여러 명의 참석자가 정해진 절차에 의해서 코드의 상세한 부분을 하나하나 검토하는 방식
  - 발생 가능한 문제를 조기에 발견할 가능성이 상당히 높지만, 수행하는 데에 드는 비용이 비싸고 빈번히 수행하기 어려우므로 빠르고 잦은 배포를 해야하는 스타트업에는 적절하지 않은 방법일 수 있음
  - 서비스 런칭 직전 또는 대규모 배포 직전에 코드 인스펙션을 통해서 좀 더 높은 품질의 서비스를 제공할 수도 있음
- 자연스럽고 가벼운 코드 리뷰(lightweight code review)
  - 복잡한 절차나 부담감 없이 자연스럽게 코드 리뷰를 수행하면서도 좋은 효과를 얻을 수 있는 방식
  - 전통적인 구분(Over-the-shorlder, Email pass-around, Pair programming, Tool-assisted code review 등)에 따라 세부적으로 나누어지기도 함
온라인 코드 리뷰
- 다양한 코드리뷰 도구가 있지만, 버전 관리 도구로 git을 사용하고 있다면 github의 pull request를 통해서 자연스럽게 온라인 코드리뷰 진행 가능
팀 리뷰
- 온라인 리뷰와는 반대로 팀 구성원들이 주기적으로 오프라인에서 진행하는 리뷰
- 특정한 기준을 미리 정해두고 형식에 맞춘 회의가 되어서는 안됨
- '자유롭고 가벼운 방식의 코드 리뷰’이기 때문에 다양한 주제를 가지고 진행 가능
- 좋은 사례 공유, 궁금했던 점 질문, 온라인 코드리뷰 과정에서 개선되었으면 하는 점 제안 등
코드 리뷰시 무엇을 확인해야 하는가?
- 기능의 정상 동작 여부
- 버그의 조기 발견
- 가독성과 유지보수 편의성
  - 소프트웨어 개발의 전체 생명주기에서 유지보수가 차지하는 비율은 매우 큼
  - 자칫 잘못하면 주관적인 견해로 옳고 그름을 따지는 비생산적 논쟁으로 변질될 수 있으므로 가급적 기준을 정하고 해당 기준으로 검토하는 것이 좋음
- 개발 표준의 준수 여부
  - 컨벤션은 가독성과 유지보수 편의성에 큰 영향을 미침
  - 형상관리 도구를 사용한다면 커밋 메세지에도 규칙을 두어 추후 변경사항 추적에 용이하게 할 수 있음
- 테스트 코드의 작성 여부
- 재사용 가능한 모듈의 중복 개발
- 배울만한 점은 없는지
  - 코드리뷰에 많은 사람이 오해하는 부분 중 하나는, 경력이 많거나 실력이 뛰어난 개발자가 후배 개발자의 코드를 검사한다고 생각하는 것
  - 코드를 리뷰할 때는 피드백을 주기 위한 시각과 좋은 점을 배우려는 시각, 이 두 가지 시각의 균형을 맞추며 진행하는 것이 좋음
코드 리뷰시 주의해야 할 점
- 코드의 맥락(CONTEXT)을 이해할 수 있는 설명 추가
  - 코드 리뷰를 받기 전에 해당 코드가 어떤 목적을 가지고 작성되었으며 왜 필요한지 맥락을 리뷰어가 이해할 수 있도록 설명을 적어두는 것이 좋음
- 하나의 이슈(버그, 기능추가)당 하나의 코드 리뷰
  - 수정사항이 너무 적다는 이유로 서로 다른 여러 개의 이슈를 동시에 처리하는 경우 발생
  - 이런 경유 리뷰어도 동시에 여러 이슈를 리뷰해야하며, 집중하기 어렵고, 리뷰 진행이 산만해지게 됨
- 리뷰 받는 코드는 한 번에 500줄 이하
  - 코드의 양(Line of Code; LOC)이 많아질수록 많은 결함을 놓침
- 주관적인 의견을 표현하는 방식 주의
- 리뷰를 너무 미루지 말자
  - 코드 리뷰에 집중할 수 있는 시간 확보 필요

코드 리뷰 in 뱅크샐러드 개발 문화 - 뱅크샐러드

"코드 리뷰란 개발자가 작성한 코드를 다른 사람들이 검토하고 피드백을 전달하며, 다시 작성자가 반영하는 과정을 말합니다."
코드 리뷰 프로세스는 팀의 규모, 업무 진행 방식, 회사의 특성에 따라 다양한 방식으로 진행됨
- 그러므로, 코드 리뷰 프로세스를 보는 것은 그 회사의 개발 문화를 이해할 수 있는 힌트가 되기도 함
비동기 커뮤니케이션
- Jira, Slack, 메일등을 활용하므로 개인의 업무시간을 존중
- 커뮤니케이션 비용을 줄임
GitHub PR을 통해 코드리뷰 진행
- Approce, Comment, Request Changes 명확하게 표현 가능
작은 PR 규칙
- 코드 리뷰 문화가 성숙해지기 전엔 LoC 규칙이 없었음
- LoC가 길어질수록 리뷰어의 집중도 저하, 이해 시간 증가, 리뷰 목표 일정 미완료 → 병목
- "1개 PR은 1,000 라인을 넘을 수 없다"
- 계속 구체화해나감
  - PR, Commit의 단위는 최소의 기능 단위로 세분화한다.
  - 테스트 코드는 제한을 두지 않음
실험 플랫폼
- 커밋이 작아지니 브랜치 전략을 커스터마이징해 실험 조직에서 다양한 실험을 해볼 수 있게됨
Low Context 커뮤니케이션을 지향하는 문화

뱅크샐러드는 저 문맥(Low Context) 커뮤니케이션을 지향합니다. 질문을 하거나 대답을 할 때, 내가 알고 있는 것을 상대방도 알고 있을 것이라는 가정을 하지 않고 충분한 문맥을 전달해야 한다는 의무를 가집니다. 상대방의 의도를 파악하기 위한 추가적인 커뮤니케이션 또는 미스 커뮤니케이션으로 발생하는 비용을 매우 비싼 비용이라는 문제의식을 갖고 있습니다.
- 리뷰의 요청과 피드백도 충분한 문맥을 제공해야 한다는 원칙을 가져감
- 작성자는 리뷰어가 사전 지식이 없는 상태에서 리뷰에 참여한다는 가정으로 모든 문맥을 제공

Note	너무 자세히? 시간 소요가 많은 PR을 작성하는 프로세스를 가져간다면 PR을 만드는 것도 스트레스라고 생각하지 않을까? "변경분을 작게 나누면 문맥을 작성하는데에도 짧게 걸리 것이다"라는 의견도 있을 듯 누군가는 문맥 작성이 오래걸리니 큰 기능을 한번에 크게 묶어서 PR을 올릴듯 PR은 큰 기능의 단위? 누군가는 이것을 작업의 단위로 볼 수도 있고, 누군가는 작게 나누어 작업의 단위로 볼 수도 있을 것

코드 리뷰 과정에서 이뤄지는 것
- 일관된 아키텍처를 유지하고 있는지
- 다른 해결 방법에 대한 의견 제시
- 버그가 발생할 가능성 제시
- 기술적인 지식, 노하우 공유
- 히스토리 전달
순리
- 빠르게 기능을 런칭하고 매출을 올리는 것 vs 개발 기간이 조금 걸리더라도 이상적인 설계대로 개발하는 것
- 뱅샐은 항상 순리를 외친다. 순리란 이치나 도리를 따르는 것. 순리대로 가는 것이 가장 빠른길임을 모두가 공감하고 지키려한다.

Note	가장 빠른 길은 제대로 가는 것이라 하지만.. 회사는 비지니스를 위한 것. 결국 변경할 수 있는 것에 한해서 순리를 지키는 것이 맞지 않을까? 그럼 "변경할 수 있는 것"은 누가 어떤 기준으로 정할 것인가? 이런게 있으면 결국 사람마다 생각하기 다름이지않을까? "변경할 수 있는 것"과 "편함을 따라가는 것" 잘 구분해야 할 듯. 개인적으로 변경할 수 있는 상황까지는 순리(?)대로 리팩토링은 해야한다고 봄