코드 리뷰

JISEUNG·2022년 3월 30일

백명석 코드리뷰

165

백명석님의 한번 듣고 평생 써먹는 코드 리뷰 노하우를 듣고 주관적인 의견을 담아 정리한 내용입니다.
본론부터 읽고 싶다면 -> 여기부터

2022 프로그래머스 개발자 설문조사 리포트

왜 코드 리뷰를 해야 할까?

우리가 살고 있는 시대

"Software is eating the world" (Marc Andreessen, 2011, WSJ)

온라인, SW가 중요한 역할을 하는 시대

Global GDP에서 IT 기술의 비율 : 2020년 5% -> 2030년 10%로 변화할 것이다.
주목할 만한 것은 나머지 90%의 Digital Transformation의 영역

4차 산업

ICT의 융합으로 이뤄지는 차세대 산업 혁명으로 좀 더 예측하기 힘들어지면서 좀 더 애자일한 방식이 필요해졌다!

개발 생산성, SW 공학의 특성, 장인 정신

개발 생산성
Release가 증가함에 따라 개발자 수가 늘어난다.
But, Release가 증가함에 따라 생산성은 떨어진다.

Why?
마틴 파울러의 설계-체력 가설(Design Stamina Hypothsis)
좋은 설계가 없으면 어느 순간부터는 기존의 기술 부채를 해결하면서 생산성이 떨어진다.

SW 공학의 특성

"The only way to go fast, is to go well" - Robert C.Martin
시간이 흘러도 생산성이 저하되고 비용 증가를 막을 수 있는 유일한 방법

설계의 산출물은 코드이기 때문에 좋은 설계는 좋은 코드(클린 코드)라고 할 수 있다.

그렇다면 좋은 개발자는..

High Quality, High Price라는 Tradeoff를 역전시킬 수 있다.

품질 높은 코드는 유지보수 비용을 낮출 수 있으므로
High Quality, Low Price로 이어지는 것!
뒤에서 더 자세히 살펴보자.

SW 장인정신

지식과 경험의 공유는 전문성을 갖춘 개발자를 육성시키는 유일한 방법이다.
그 방법 중 하나가 바로 Code Review
결함 없는 SW 배포와 동시에
개발자가 지금 당장 할 수 있는 공유 활동

코드 리뷰의 정의와 목적

정의

Code Review is a software quality assurance activity in which one or several people check a program mainly by viewing and reading parts of its source code, and they do so after implementation or as an interruption of implementation
Peer reviews, Pull request, Merge request

목적

주목적

품질 문제 검수 : 버그, 장애

부수적인 목적 및 효과

더 나은 코드 품질 : 아키텍처 속성 개선을 위한 코드 개선 (향후 변경 비용 개선)
학습 및 지식 전달의 수단 : 코드, 해결책 등과 관련된 지식 공유에 기여
공유(주고받는 학습)를 통한 역량 증대 및 성장
Hard Skill : 개발의 구체적인 영역들
Soft Skill : 커뮤니케이션 방법
동기 부여 및 상호 책임감 증대 : 집단 코드 오너십 및 결속 증대. 팀워크, 개발 문화 개선

개발 생산성 vs 개발 품질의 Tradeoff
버그 수정 비용은 개발 라이프사이클(개발, 테스트, 배포 등) 후반으로 갈수록 기하급수적으로 커진다.
품질이 높으면 라이프사이클 후반으로 갈수록 시간 절약
따라서 TDD, Test, Refactoring, Code Review 등은 생산성을 증대시킨다.
높은 품질은 향후 추가 비용을 감소시킨다.

코드 리뷰의 절차

변경 내역(Change List, PR)

리뷰 시작 전에 작성. 저자가 Merge를 원하는 소스 코드에 대한 일련의 변경(잘한 것, 아쉬운 것, 눈여겨볼 것)에 대해 기술

PR에 다이어그램, 링크 등을 걸어도 좋다. 대충 적지 말자.

왜 코드 리뷰가 어려운가

What

지식/공학적 결정을 공유하는 기회
공유(잘한 것, 아쉬운 것)를 통해 서로의 지식/경험을 나누며 상호 학습을 통한 역량 증대 수단

Why
코드 토의를 자신에 대한 비판으로 받아들이면 물거품..

생각을 글로 전달하는 것에 대한 어려움
오해의 위험이 큼(음성 톤, 표정의 부재)

How
피드백을 조심스럽게 표현하는 것이 더 중요하다.

코드 리뷰 기법들

효율적인 PR 방법

1. 지루한 작업은 컴퓨터로 처리하라. 리뷰어가 하게 하면 안 된다.
코드를 읽는 것은 인지적 부담. 고수준의 집중이 필요하다.
컴퓨터가 할 수 있는 일에 이런 노력을 낭비하게 하지 말자
ex. Formatting Tool 공백, 들여쓰기 오류 등

format은 별도의 커밋/PR로 분리해서 리뷰를 생략할 수 있도록 하자
cf. 이런 방법도 좋을 것 같다. Git Hooks 등 별도의 툴을 사용하자

2. 스타일 가이드를 통해 스타일 논쟁을 해소
스타일에 대한 논쟁은 리뷰에서 시간 낭비다. 이렇게 하자

Option
1. Adopt an existing style guide
2. Create your own style guide incrementally
3. The Hybrid approach

3. PR을 올릴 때 주석 달기
PR을 올릴 때 저자가 첫 번째 주석(Comment)을 다는 것이 도움이 된다.
PR을 저자가 먼저 읽어 보고 -> 리뷰어들을 위한 설명을 Comment로 남겨서 -> 리뷰어들의 시간을 절약할 수 있게 하라

4. 리뷰어에 가능한 모두를 포함하라

많은 사람이 볼수록 버그를 더 잘 찾아낼 수 있다.
많은 사람이 본다는 것을 알면 사람들은 대개 더 잘하려는 경향이 있다. (코드 작성, PR 작성 모두에서)

5. 의미 있는 커밋으로 분리해라
혼자 하는 코드 리뷰를 생각해보자.
아주 단순한 코드가 아니라면 내가 짠 코드라도 2주 후에 보면 남의 코드다.
커밋 로그, PR을 혼자서도 잘 작성해 놓으면 2주 후의 나에게도 도움이 된다.

효율적인 리뷰 방법

1. 리뷰는 즉시 시작
코드 리뷰를 높은 우선순위로 두자
저자는 리뷰가 종료될 때까지 대기(Blocked)하는 상태다

코드를 읽고 피드백을 줄 때는 시간을 둬도 되지만, 시작은 바로 해라. 이상적으로는 수 분 내로
리뷰 라운드의 최대 시간은 하루
다른 우선순위가 있다면 다른 리뷰어를 지정해라
월 1회 이상 재지정을 해야 한다면 속도를 줄여서 건강한 개발 관습(Practices)을 유지할 수 있어야 한다.

"If it hurts, do it more often" (Agile pull requests, Mark Seemann)

매일 아침 30분, 점심 식사 후 30분을 리뷰를 위해 미리 확보

PR에 포함된 변경이 적도록 노력 (반나절 정도의 작업량)

2. 고수준으로 시작, 저수준으로 내려가라
리뷰 라운드에서 많은 의견을 남길수록 저자가 당황할 위험이 커진다.
하나의 라운드에 20+ 의견은 위험의 시작

초기 라운드에서는 고수준 피드백으로 제한
버그, 장애, 성능, 보안
Extract Method, Composed Method, Invert-if(복잡도) 등
고수준 피드백이 처리된 후에 저수준 이슈를 처리
(선택적인) 설계 개선
변수명 변경, 주석을 명확하게 하는 것 등

3. 예제 코드 제공에 관대해라
저자를 기분 좋게 하기 위한 선물
과하면 저자가 코드를 작성할 수 없다고 생각한다는 신호

너무 긴 예제는 관대가 아니라 억압
라운드당 2~3개로 제한

4. 리뷰의 범위를 존중하라

자주 보이는 Anti-pattern : PR 근처의 코드를 보고 저자에게 수정 요청
Rule of Thumb : PR에 포함되지 않은 라인은 리뷰 범위가 아님
예외 : pr이 둘러싼 코드에 영향을 미칠 때

5. 태그를 활용

[Nit]
고치면 좋지만 아니어도 그만(nitpick)의 약자

리뷰어는 더 개선할 의견을 자유롭게 남길 수 있어야 한다.
중요치 않다면 태그를 남겨서 저자가 무시할 수 있도록 하자
개발자들이 지속적으로 기술을 연마할 수 있도록 돕는 것이 목적이다!

cf. 개인적으로 좋아하는 방법 : Pn 룰

6. 한두 등급만 코드 레벨을 올리는 것을 목표로

Letter Grade

D 등급의 PR을 받으면 저자가 C나 B를 받도록 도와라
완전하지는 않아도 충분히 좋은 코드가 되도록

F : 기능적으로 틀렸거나 너무 복잡해서 정합성에 확신이 없는 상태

승인을 보류하는 유일한 이유 : 여러 차례의 리뷰 라운드 후에도 코드가 F 상태인 경우. 도저히 배포할 수 없는 경우..

피드백 방법

1. 절대 '너'라고 하지 마라. (너는 왜..)
리뷰의 핵심은 '무엇이 코드를 나아지게 하는가'이지 누가 무슨 잘못을 했는지가 아님

비판의 대상은 '코드'인데 상대를 지칭함으로써 주의를 코드 -> 저자로 돌리면 안된다.
(X) You misspelled "successfully"
(O) "sucessfully" => "successfully"
I Message 대화법 (행동-결과-감정)
오픈 커뮤니케이션 : ~하는 것을 제안합니다. ~하는 게 어떨까요?
물어보면 대답하지만 안 한다고 대답해도 된다.

2. 건설적인 피드백을 하라
동료들 간의 코드 리뷰 : 경쟁 유발이 아니라 팀의 생산성 높이자
비판이 아니라 프로젝트 성공에 기여하도록 하는 학습의 과정
실수에서 배우고 역량을 증대하도록 동기부여

건설적으로 못 하겠다? 침묵하자. 어설프게 기분 망치지 말자.

3. 진정한 칭찬을 해라
대부분의 리뷰어가 잘못된 부분에만 집중한다.
리뷰는 긍정적 행위 강화를 위한 기회기도 함

이런 API가 있었나요, 유용해요
정말 좋은 해결책이네요. 생각도 못 했어요.
함수를 분리한 것은 좋은 생각이에요. 훨씬 단순해졌어요.

저자가 주니어/신규 입사자인 경우 특히 리뷰에 민감하고 방어적일 수 있다.
리뷰어가 감시자가 아니라 도와주려는 팀 동료임을 보여줘서 긴장감을 낮출 수 있다.

4. 명령이 아니라 요청

How를 알려주는 것이 아니라 What을 요청하라

명령(How) : 구체적으로 어떻게 하라고 명령하는 것. 저자의 자율성을 억압
요청(What) : 선언형. Tell, don't ask

5. 의견이 아니라 원칙 기반
제안하는 변경, 변경의 이유를 모두 설명해라

이 클래스를 2개로 분리해야 해요
-> 지금 이 클래스는 파일 다운로드와 파싱의 2가지 책임을 가지고 있어요.
다운로더와 파서 2개의 클래스로 분리하여 SRP를 준수하는 것이 어떨까요?

SW는 과학인 동시에 예술이기도 하다. 단지 그냥 보기 싫거나 직관적이지 않을 수 있다.
이때는 I Message를 써보자.

이 코드는 혼란스럽네요.
-> 나는 이 코드를 이해하기 어렵네요.

6. 반복적인 패턴에 대해 피드백을 제한해라
동일 패턴에 대해 2~3개 정도의 예를 언급하라
개별 사례가 아니라, 패턴에 대해 수정을 요구하라
~~반영이 안되면? 어쩔 수 없다..~~

교착상태(Deadlock)

교착상태 알아채기

라운드당 Comment가 줄어들지 않는 경향
너무 많은 Comment에 저항

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

만나서 얘기하라!
화상/만나서 논의 (특히 복잡한 리뷰)
텍스트 기반 의사소통은 상대가 인간이라는 것을 잊게 한다.
설계 리뷰를 고려하라
코드 리뷰 때 설계 리뷰 때 논의되었어야 할 사항을 논쟁하는가? 그런 거라면 넘어가야 한다.
인정하자 ~~그냥..~~
교착상태가 길어지면 관계가 나빠진다. ~~퇴사까지,,~~
Agree to disagree. 그냥 승인하는 비용

저수준 코드를 무심코 승인하면 SW 품질이 낮아질 수 있지만,
너무 다퉈서 함께 일하지 않게 된다면 고수준의 품질을 얻을 기회조차 없어진다.
아주 심각하지 않으면 그냥 인정하고 좋은 관계로 협업하는 것이 현명하다.
인정이 불가한, 너무 크리티컬한 경우
저자에 대한 논의를 팀장이나 테크 리더에게 escalation
다른 리뷰어에게 할당

교착상태로부터 회복하기

상황을 관리자와 논의
휴식. 가능하다면 안정될 때까지 서로 PR 보내지 마라.
갈등 해결책(Soft skill)을 학습하자

좀 맘에 안 들지만 순응하고 살 수 있다.
그렇게 살면 행복해질 수 있을까?
그럼 나를 고쳐볼까?

Appendix

`Pull Requests` vs `Pair Programming`?

Tradeoff : Latency or Throughput

내성적, 사색, 비동기? Pull Requests
외향적, 친밀한 개인적 상호 작용? Pair Programming
답은 없다. 앙상블 방식도 좋다.
이건 나중에 글로 표현하기 어렵겠다 싶으면 그 부분은 Pair Programming

코드 리뷰 문화 정착의 어려움, 극복 방법

근본적인 문제는 사람들이 리뷰할 시간이 없다고 느낀다는 것

조직에서 개인 기여로만 평가를 받는다면,

커밋을 하고 PR을 날리는 것은 개인 기여로 평가받지만, 팀을 돕기 위해 수행하는 일은 시간 낭비처럼 보임
이것은 조직적인 문제 ~~이지만 극복해야겠지..~~

극복하기

리뷰어 N명의 시간 절약하기 위한 저자의 노력이 중요하다.
"저자"가 고생해서 리뷰어의 시간을 아껴줘야 한다.

cf. 개인적으로 깃헙의 gh-pages를 이용해 preview를 제공하는 방법을 도입해보고 싶다.
리더의 관심과 의지
가끔, 그러나 매우 자세히
코드 비난의 두려움 피하기

"How do you inspire your team to adopt that mentality?" (craftmanship, TDD, Pair Programming)
- 좌절할 준비를 하라
- 자신의 태도만 제어할 수 있다. 타인이 어떻게 반응할지는 제어 불가
- 영감을 주는 것은 부산물 (side effect)

어떻게 자타의 모범이 되어 영감을 줄 수 있을까?
지각하지 말라 업무에 기여해라 긍정적으로 임해라 사람들에게 잘해라
기술적 훈련(technical discipline)을 언급하려면 고도로 숙달되어야 한다.
키보드로 코딩을 할 때 사람들이 경악하도록 해라.
이 정도 되면 영감을 줄 수도.. ?

코드 리뷰를 잘하기 위해 필요한 기술들, 책 추천

1. 리팩토링

결과의 변경 없이 코드의 구조를 재조정하여 가독성을 높이고, 유지보수를 편하게 하는 것.
버그를 없애거나 새로운 기능을 추가하는 행위는 아니다.

리팩토링에서 다루는 많은 것들이 코드 리뷰에서 언급된다.

TDD와 연결된 리팩토링
돌아가는 코드로 설계를 하는 행위

매일 할 수 있는 리팩토링

Composing Methods(Extract Method)로 전이될 수 있게 하자.

응집도 높이기 : 의존성 낮추기. 결합도 낮추기는 어렵지만, 응집도를 높이는 건 상대적으로 쉽다.

Discover Value Object

2. Legacy Code 다루기

의존성 관리 : Subclass & Override Method
테스트 추가 : Characterization Test
새로운 코드 빠르게 추가 : Sprout/Wrap Method/Class
레거시 분석

3. Clean Code & TDD

Clean Code 클린 코드, 클린 코드 강의
단위테스트 전통파(Classic) and(not vs) 런던파(London)
TDD Outside in -> Inside out

Kent Beck의 TDD에 대한 의견
fail->refactoring.. clean..
코드 ~~드럽게~~ 만드는 사람이랑 리팩토링해서 깨끗하게 만드는 사람이 똑같은 사람인데 왜 그렇게 하냐?
한 번에 두 가지를 같이 하기 힘들다. 빨리하는 거랑 잘 만드는 거랑 배타적이다. 잘 안된다.
일단 만들고 테스트 커버리지, 코드 리뷰로 높여 나가자.

일단 이렇게 시작해보자

일단 시작하고, 방법을 찾고, 학습하면서 내 몸에 익혀서 발전시켜라

확인해라 : 상태를 공유하고 피드백을 받아보자
너무 기법/절차에 의지하지 말라 : 온·오프를 적절히 섞어라. 무슨 툴을 썼는지보다 잘 읽히는 코드인지
일정, 우선순위 : 시간이 정해진 시험에서 성적을 평가받는 것. 시간을 아끼는 방법은 저자의 노력
주기 : 짧게, 자주, PR 사이즈는 작게
단위가 큰 PR은 커밋이라도 잘 나누자
커밋 단위로 봐달라고 의견을 주자
feature/branch 개발이 끝나지 않았으면?
: 너무 많은 변경을 하고 있어서 리뷰어가 힘들 거 같다면 같은 브랜치라도 나눠서 PR 올려도 괜찮다.

코드 리뷰 예시 소개