Who?

같은 프로젝트, 같은 성격을 갖고 있는 프로젝트를 진행하고 있는 개발자

어느 아이디어가 강점이라 생각되는지, 혹은 그 반대인지 소통한다.
문제를 해결할 때까지 코드를 단순화하는 것으로 방향을 찾아라.
:thumbsup: 또는 “머지 준비됨 Ready to merge”와 같은 코멘트와 함께 풀 리퀘스트를 수락한다.

Why?

코드 리뷰는 성숙한 개발 조직의 문화, '문화'란 특별한 이유없이 구성원이 모두 동의했을때, 그것을 문화라고 한다

코드 리뷰는 성장하고자 하는 개발자의 권리, 조직으로부터 권리를 보장받을 수 있어야 함

코드 리뷰는 잔소리가 아닌 코드로 하는 커뮤니케이션

How?

많은 규칙보다는 필요한 규칙만 정한다

feature/***/ --> develop` merge 시 Pull Request를 통해 리뷰를 요청한다.

깃 플로우 기반 + Pull Request 활용
- Git-Flow 기반의 코드리뷰
  
  개발자가 개발에 착수하면 이슈 트래커의 상태를 ‘In Progress’로 변경하고 코드 작업을 진행합니다. 현재 버전 관리 시스템으로 Git을 사용하고 있기 때문에 git-flow 기반으로 아래와 같은 브랜치 전략을 만들어서 활용하고 있습니다.
  
  https://engineering-org.line-apps.com/ko/lineadsdevops4/
  - 개발(develop): 모든 개발 작업물은 이 브랜치를 기준으로 체크아웃(checkout)하고, 머지(merge)합니다.
  - 피처(feature): 단위 기능을 개발하는 브랜치입니다. 브랜치를 생성할 때 이슈 트래커의 번호를 이용해서(예: feature/LADS-2797) 결과물을 쉽게 추적할 수 있고 머지를 완료한 브랜치를 주기적으로 정리할 수 있습니다. 작업을 완료한 결과물은 코드 리뷰를 통과해야만 개발 브랜치로 머지할 수 있습니다.
  - 마스터(master): 모든 개발 작업물이 배포를 위해 마지막으로 모이는 브랜치입니다. 개발 브랜치의 결과물을 마스터 브랜치로 머지한 직후 배포를 위한 새 태그를 생성합니다. 태그는 v(메이저 버전).(YYMMDD).(당일 배포 횟수)와 같은 형식으로 생성하고 있습니다(예: v1.210416.0).
  - 핫픽스(hotfix): 배포까지 완료한 개발 작업물에서 버그가 발생해 긴급 대응이 필요한 경우 이 브랜치를 생성해서 바로 마스터 브랜치로 머지합니다.
리뷰 없이는 머지되지 않도록 제한필요

master, develop 브랜치로는 바로 푸시하지 못하도록 push 깃훅 추가

프로젝트를 클론할 때, 깃훅을 반드시 설치하도록 강제함
- Git hook 사용법 안내글
컨벤션 리뷰는 commit 깃훅에 린트 작업을 추가해 해당 커밋에서 변경이 발생한 파일에 대해서만 린트검사 수행가능 (점진적 정리가 가능)
유익하다고 느낀 리뷰

미리 발견하는 버그

기존 코드의 히스토리

삽질 회피 코드의 공유

더 나은 로직의 제안

더 나은 변수명 제안
리뷰이는 리뷰에 어떻게 반응하면 좋을까?

리뷰어의 의견은 잘 청취하고, 의도를 잘 설명하되, 결정은 본인이 하는 게 좋다고 생각함

리뷰어가 그 코드가 정 마음에 들지 않았다면, 다음 기회에 직접 수정하는 걸로
리뷰가 병목이 되는 상황을 어떻게 개선할수 있을까?

모든 변경건에 대해 리뷰를 요청하면 바쁜경우 리뷰가 계속 쌓임 → 테스트가 늦어짐 → 통합테스트에서 버그 발견ㅜ

브랜치 미리보기 서버로 해결

깃헙의 gh-pages에서 아이디어를 얻음
- 공유하고자 하는 브랜치를 특정 패턴의 이름으로 푸시하면, 해당 기능의 스냅샷이 배포되도록 서버를 구성함
- 예를 들어, 개발자는 작업하던 feature/XXX 브랜치를 preview/XXX로 푸시하면 됨
- 푸시 후엔 XXX.story.kakao.com 같은 URL로 공유할 수 있음
리뷰 마스터
- 주기적으로 리뷰를 독려하고 머지를 담당하는 ‘리뷰 마스터’ 제도 도입
- 리뷰 마스터는 한 주 또는 특정 주기로 쌓여있는 PR을 빨리 리뷰하도록 처리하고,
- 스펙이 큰 피처인 경우엔 리뷰를 단계별로 나누어서 진행하는 것도 좋음
- 팀원들에게 코드리뷰를 상기시켜라
최대한 친절하게 격려하며! 최대한 구체적으로 자세하게!

1.1 모두에게 해당

대다수 프로그래밍에서 내려진 결정은 각각의 견해에 따른다는 사실을 받아들인다. 어느 쪽이 더 좋은지 장단점을 의논하고 빠르게 결정을 내린다.
질문한다. 대신 답을 강요하지 않는다. (“이 :user_id라는 네이밍에 대해 어떻게 생각하나요?”)
코드의 소유권에 대해 특정하는 것을 피한다. (“내가 작성한”, “내 코드가 아닌”, “당신이 작성한”)

1.2 내 코드를 리뷰 받을 때

리뷰어의 추천에 감사해야 한다. (“리뷰 고맙다. 그렇게 변경하도록 하겠다.”)
개인적인 부분으로 받아들이지 않는다. 리뷰의 대상은 코드지 당신이 아니다.
초기 피드백을 받을 때는 독립적인 커밋으로 만들어 브랜치로 푸시한다. 브런치가 머지되기 전까지 커밋을 뭉쳐버리지 않는다. 리뷰어는 초기 피드백에 기반을 둬 작성한 각각의 업데이트를 읽는 것이 가능해야 한다.
모든 코멘트에 응답하도록 노력한다.

1.3 코드를 리뷰할 때

왜 이 코드가 필요한지 이해한다. (버그, 사용자 경험, 리팩토링.) 그러고 나서:

좋은 커밋 메시지는 패치에 관한 다음 세 가지 질문에 답을 할 수 있어야 한다:

왜 이 코드가 필요한가? 코드는 버그 수정, 기능 추가, 성능 향상, 신뢰성, 안정성을 위한 변경일 수 있다. 물론 단순한 오·탈자 교정일 수도 있다.
어떻게 이슈를 해결했는가? 짧아서 명백한 패치의 경우 이 부분을 생략해도 된다. 다만 깊은 수준의 묘사로 어떻게 문제에 접근했는지 나타내야 한다.
패치가 어떤 영향을 만드는가? 명백한 부분을 포함해 벤치마크 결과, 부작용 등을 포함할 수 있다.

커밋 메시지의 구조

(50자 미만의) 변경에 대한 짧은 요약

필요하다면 상세한 설명을 첨가한다. 행 당 72자가 넘지 않도록 유의한다. 맥락에 따라 첫 줄은 이메일의 제목처럼,
나머지는 본문처럼 다뤄지는 경우가 있다. 요약과 본문을 구분하는 빈 행은 본문을 생략하지 않는 이상 필수적이다.
rebase와 같은 도구를 사용하면 혼란을 줄 수 있기 때문이다.

추가적인 문단은 빈 행 다음에 작성한다.

- 개조식 서술 또한 괜찮음

- 블릿(bullet)으로 하이픈(-)이나 별표(*)를 사용하고, 한 칸의 공간을 띄고 각 항목 사이 빈 행을 넣는
  방식이 일반적이나 다양한 관례가 있음.

“고쳤다 fixed”, “추가했다 added”, “변경했다 changed” 대신 “고치다 fix”, “추가하다 add”, “변경하다 change” 식으로 작성한다.
항상 두번째 빈 행을 추가한다.
커밋 메시지에서 줄 바꿈을 한다.
요약 끝에 마침표를 찍지 않는다. 요약은 제목이다. 제목은 마침표로 끝나지 않는다.

예제 코드 제공에 관대해라

절대 "너"라고 하지마라 → "너"를 "우리"로 변경하라

피드백은 명령이 아니라 요청으로 표현해라

ex.

Foo 클래스의 별도의 파일로 분리하라 →
Foo 클래스를 별도의 파일로 분리할 수 있을까요?

의견이 아니라 원칙에 기반하여 피드백하라

제안하는 변경과 변경의 이유를 모두 설명하라

리뷰의 범위를 존중하라

Changelist에 포함되지 않은 라인은 리뷰 범위가 아님

진정한 칭찬을 해라

진심어린 칭찬은 리뷰어가 잔인한 감시자가 아니라 도와주려는 팀동료라는 것을 보여서 이런 긴장감을 낮춰준다

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

코드 리뷰의 최악의 결과는 교착상태

만나서 얘기하라

설계 리뷰를 고려하라

인정하거나

인정이 불가한 경우

논의를 팀장이나 테크 리더에게 Escalation 권유

다른 리뷰어에게 할당을 권유

시멘틱 버전과 컨벤셔널 커밋

시멘틱 버전이란 소프트웨어 패키지의 버전을 관리하는 프로토콜을 의미한다.

{MAJOR}.{MINOR}.{PATCH}

로 나누어

기존 버전과 호환되지 않게 API가 바뀌면 “MAJOR 버전”을 올리고,
기존 버전과 호환되면서 새로운 기능을 추가할 때는 “MINOR 버전”을 올리고,
기존 버전과 호환되면서 버그를 수정한 것이라면 “PATCH 버전”을 올린다.

fix: correct MINOR typos in code

see the issue for details
on typos fixed.

Reviewed-by: Z
Refs #133

가장 첫번째 줄에 fix: 키워드를 통해 해당 커밋은 버그 수정임을 알 수 있다.

build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
docs: Documentation only changes
feat: A new feature
fix: A bug fix
perf: A code change that improves performance
refactor: A code change that neither fixes a bug nor adds a feature
style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
test: Adding missing tests or correcting existing tests

PATCH 업데이트

* 8fffafb fix: somethin
* c87775a refactor: something

위와 같은 커밋메시지는 PATCH 버전의 변화를 의미한다. 새로운 피쳐(feat)가 추가 되지 않았기 때문이다.

MINOR 업데이트

* 8fffafb feat: create a new helper function
* c87775a fix: remove a condition

위 커밋 메시지들을 살펴보면 MINOR 버전의 변화를 의미한다. 새로운 피쳐의 추가를 의미하는 feat 키워드가 사용되었기 때문이다.

MAJOR 업데이트

그럼 MAJOR 버전의 변화를 의미하는 커밋 메시지는 어떻게 생겼을까?

아래와 같이 BREAKING CHANGE 라는 키워드를 커밋 메시지에 추가해준다.

feat: allow provided config object to extend other configs

BREAKING CHANGE: `extends` key in config file is now used for extending other config files.

일관된 커밋 메시지를 사용함으로써 CHANGELOG 또한 자동으로 생성할 수 있게 된다. 시맨틱 릴리즈를 사용하면 앞으로 시맨틱 버전과 함께 CHANGELOG 또한 자동으로 생성된다.

참고자료

https://github.com/JaeYeopHan/tip-archive/issues/13

https://tech.kakao.com/2016/02/04/code-review/

https://www.slideshare.net/JiyeonSeo2/ss-73455188

https://edykim.com/ko/post/code-review-guide/

https://www.slideshare.net/codetemplate/2018-01code-review-95601233

컨벤셔널 커밋을 이용한 CHANGELOG 및 시멘틱 버저닝(Semantic Versioning) 자동화

LINE 광고 서버 개발 팀의 DevOps 문화

parksunwoo / memo-archive