[정보] 카카오스토리 웹팀의 코드리뷰 경험

2016.08.08 KAKAO 사내 강연
2016.07.20 NHN 엔터테인먼트 초청 강연 @플레이뮤지엄
2016.04.21 네이버 테크토크 초청 강연 @그린팩토리
관련 자료입니다.

카카오스토리 웹팀의 코드리뷰 경험 from Ohgyun Ahn

 

 
1. 카카오스토리 웹팀의 코드리뷰 경험 안오균
2.  @네이버 건강, 뉴스, 애널리틱스, 포토앨범, N드라이브, 디자인 스튜디오, nChart @카카오 카카오스토리 웹 / iOS
 3.  오늘은, • 코드리뷰 도입부터 안정화까지 • 코드리뷰에 대한 회고 • 우리 팀에 코드리뷰 도입하기 • Q&A + 부록
4.  코드리뷰 도입부터 안정화까지 (리뷰 병목 극복기)
5.  story.kakao.com
 6.  카카오스토리 웹 • 프론트엔드 + 웹 서버 • JavaScript 95%, nginx, Bash • Backbone 기반 자체 프레임워크
7.  코드리뷰 도입 • 2013년 10월 시작, 멤버는 3명 • Git Flow + Github Pull Request 활용 • 모든 코드 리뷰 원칙
8.  Git Flow • Git 브랜칭 도구 • 모든 피처는 develop 브랜치에서 시작 • feature/xxx 패턴으로 작업 후 develop으로 머지
9.  Pull Request (PR) • Github의 기능 중 하나 • 한 브랜치를 다른 브랜치로 머지하도록 요청 • 온라인 리뷰가 가능하고, • 온라인에서 머지 가능
10.  리뷰 규칙 • 피처 브랜치(feature/xxx)에서 작업하고 • develop 브랜치로 Pull Request • 모든 멤버가 동의했을 때 머지
11.  예외적인 상황이 발생됨 • 실수로 develop 브랜치에 푸시하는 경우 • 간단한 수정이라고 바로 develop으로 푸시
12.  리뷰 없이 머지할 수 없도록 제한 • pre-push 깃훅 활용 • master, develop 브랜치로 푸시하지 못하도록 함
13.  Git Hooks • 특정 이벤트가 생겼을 때 실행하는 스크립트 • 클라이언트 훅 / 서버 훅 • 커밋 전/후, 푸시 전/후 등 • 스크립트 실패하면 이벤트 중단
14.  pre-push 깃훅의 효과 • 모든 피처가 Pull Request를 통해서 머지됨 • 초기 리뷰 문화 정착에 도움 • 이 단계부터 ‘리뷰는 필수’라는 인식이 자리잡음
15.  코드 스타일에 대한 리뷰가 대부분 • 탭, 공백, 들여쓰기, 캐멀케이스, 언더스코어 • 스타일 가이드 + 에디터 포맷터는 존재 • 하지만 잘 지켜지지 않음
16.  코드 스타일 리뷰는 피곤함 • 코드 스타일을 통일하면 참 좋겠지만 • 남기는 사람도 보는 사람도 불편함 • 도구로 해결할 수 없을까?
17.  pre-commit 깃훅에서 린트 수행 • 해당 커밋에서 변경한 파일 대상 • 린트 실패 시 커밋할 수 없음
18.  pre-commit 훅의 린트는 성공적 • 코드 스타일에 대한 리뷰가 크게 감소함 • 수정하는 파일만 대상이므로 거부감이 적었음 • 대상 파일의 나머지 코드도 수정해야 함 • 수 개월 후 프로젝트 린트 오류 0%
 19.  PR 규모가 커서 리뷰하기 어려움 • 프로젝트 초기, 기초 구조를 잡던 때 • 코드량이 많고 커밋의 단위도 커 리뷰하기 어려움 • 전체 흐름을 파악하기도 어려움
20.  오프라인과 온라인 리뷰를 병행 • 오프라인 리뷰에서 전반적인 의도를 설명 • 프로젝터와 화이트보드 활용 • 회의 종료 후 온라인 리뷰
21.  PR과 커밋 단위에 대한 합의 • PR은 피처 단위로 • 커밋은 의미있는 작업 단위로 • 개발 정기 미팅 때 합의
22.  우리가 계획한 프로세스 • 피처 작업 > develop 머지 > 알파 배포 > 테스트 • 알파 서버는 develop 브랜치를 바라봄 • 이터레이션 테스트는 알파 서버에서 수행
23.  바쁠수록 리뷰를 미루게 됨 • 피처 작업하기 바쁘니 리뷰를 미루게 됨 • 새 리뷰 대상은 더 늘어남 • 쌓여있는 PR이 10개가 넘는 경우가 생김
24.  리뷰시간이 예상보다 오래 걸림 • 큰 피처인 경우 1시간 이상 • 리뷰가 많으면 하루를 다 쓰는 경우도 있음 • 태스크 관리 도구에 ‘개발 리뷰 중’ 단계를 추가
25.  리뷰가 병목이 됨 • 피처 작업은 완료해도 리뷰 대기 중 • 리뷰가 되지 않아 develop 으로 머지하지 못함 • 대상 피처가 알파 서버에 배포되지 못함
26.  기획/디자인 직군의 불만 • 타직군은 알파 서버에서 피처 확인 가능 • 개발은 완료됐다고 하는데, • 리뷰가 안돼서 확인할 수 없다고 함
27.  통합 테스트 때 스펙 변경이 발생함 • 개발 완료 시점에 모든 피처가 한꺼번에 머지됨 • 타직군은 구현된 기능을 이제서야 볼 수 있음 • 버그 뿐 아니라 스펙과 디자인 변경이 다수 발생
28.  서로에 대한 불만 • 개발: 내일 모레가 배포인데 스펙 변경? • 기획/디자인: 개발 다 됐다면서 이제 첨 보여줌?
29.  리뷰 병목의 폐해 • 이 시점이 피로도가 가장 컸음 • 야근이 많아지고 의욕도 떨어짐 • 리뷰의 병목이 그 원인 중 하나
30.  기획/디자인 직군이 확인하고 싶었던 것 • 구현된 기능의 사용성 테스트 • 기획과 디자인 의도가 잘 반영되었는지 • 서비스에 적용했을 때 실제로 무리가 없을 지
31.  기획/디자인 직군이 확인할 수 있는 방법 • 개발자 자리에서 확인 • 개발자 로컬 서버에서 확인 • 일정에 쫓기고 피처가 많다보니 불편함
32.  브랜치 미리보기 서버를 구성함 • 깃헙의 gh-pages 에서 아이디어를 얻음 • 개발자는 특정 패턴의 이름으로 푸시하면 됨 • 각 피처 브랜치의 스냅샷을 배포
33.  미리보기로 띄우기 • 피처 브랜치: feature/example • $ git push origin feature/example:preview/example • example.story.kakao.com 으로 공유 가능
34.  미리보기 서버 동작 원리 • 주기적으로 리파지터리를 fetch • fetch 아웃풋을 파싱해서 변경된 브랜치 빌드 • 빌드된 브랜치 디렉토리를 서브도메인으로 할당
35.  미리보기 서버는 매우 성공적 • develop 머지 전(리뷰 전) 피처 공유 가능 • 기획/디자인의 피드백을 빠른 시점에 받고 • 통합 테스트 때 스펙 변경이 크게 감소 • 아이디어 프로토타입 공유 용도로 활발히 사용
36.  프로젝트 멤버가 늘어남 • 최대 8명까지 늘어남 • 대부분 리뷰 문화가 거의 없던 팀에서 온 멤버 • 잘 적응할 수 있을까?
37.  우리 팀은 코드 리뷰를 하는 팀 • ‘리뷰는 당연하다’는 문화는 정착된 상태 • 영입 전부터 코드 리뷰 문화에 대해 강하게 언급 • 첫 PR부터 폭풍 리뷰
38.  233 216 288 리뷰 댓글이 200개가 넘는 Pull Request
 39.  새 멤버들의 공통된 리뷰 후기 • 초기의 리뷰는 스트레스였다 (특히 코드 스타일) • 코드 학습 효과가 좋았다 • 시간이 지나니 코드 스타일에 익숙해지더라
40.  생산되는 코드만큼 리뷰 대상도 늘어남 • 멤버가 늘어나니 생산되는 코드량도 늘어남 • 모든 코드를 보기엔 시간이 부족함 • 모든 팀원이 리뷰하길 기다릴 수 없음
41.  리뷰 팀을 구분 • 팀을 두 개로 나눠 리뷰를 진행하기로 함 • 필요하다면 별도의 리뷰어 지정 • 이제는 리뷰하지 못하고 머지되는 코드들도 많음
42.  여전히 리뷰는 지연됨 • 일정에 쫓길수록, 피처 작업 때문에 리뷰가 밀림 • 테스트 날짜가 다가오지 않으면 리뷰하지 않음 • 바쁜 한 사람이 리뷰하지 못해 머지되지 않음
43.  테스트 직전 리뷰가 몰림 • 작업은 일주일 전에 했는데, 리뷰 피드백은 이제 • 버그 수정도 바쁜데 리뷰 피드백까지 해야함 • 컨텍스트 전환 비용이 큼
44.  리뷰 데이 도입 • 매주 정해진 요일을 리뷰 데이로 선정 • 최우선으로 쌓여있는 PR의 리뷰 진행
45.  리뷰 데이의 효과는 미미 • 처음 몇 주까지는 잘 지켜지는 것 같았음 • 본인 PR이 많으면 피드백하느라 리뷰 집중 어려움 • ‘바쁜 한 사람’ 이슈는 해결되지 않음 • 바빠질수록 리뷰는 우선순위가 떨어짐
46.  리뷰 마스터 제도 도입 • 주기적으로 리뷰를 독려하고 머지하는 역할 • 한 주 또는 특정 주기로 PR을 머지하는 책임 • 개인 판단 하에 머지 가능한 것은 빠르게 머지
47.  리뷰 마스터는 성공적 • 개인에게 할당된 책임 때문에 잘 동작함 • 애매한 경우 결정권이 있어 의사결정에 빠름 • 바쁜 한 사람 이슈 해결
48.  여러 사람이 담당하는 피처의 리뷰 • 한 피처를 여러 명이 함께 작업하는 경우 • 작업 범위가 겹쳐 develop으로 PR 애매함 • 피처 단위가 커서 한 번에 리뷰하기엔 부담스러움
49.  메인 피처 브랜치로 PR하도록 함 • 피처의 메인 브랜치인 feature/A를 따고 • 하위 피처를 feature/A-1, feature/A-2로 작업 • 작업 후, feature/A-1 > feature/A 로 PR
 50.  2번에 걸쳐 리뷰함 • 상위 피처 브랜치로의 1차 리뷰는 담당자끼리만 • 1차 리뷰는 큼직한 구조나 로직에 대해 러프하게 • develop 브랜치로의 2차 리뷰는 모두가 참여
51.  2차 리뷰의 효과 • 구조 변경에 대한 피드백이 1차 리뷰에서 가능 • 테스트 직전에 큰 변경이 적어짐 • 두 번째 리뷰부턴 확실히 속도가 빠름
52.  서비스 운영 모드 • 오픈 전엔 이터레이션 별로 태스크를 묶어 배포 • 오픈 후 운영 모드로 전환 • 계획보단 실행에 포커스
53.  짧아진 배포 주기 • 구현되는 피처는 가능한 빠르게 배포하기로 함 • 배포 주기: 0.5~3일 • 리뷰 마스터 제도가 자연스럽게 사라짐
54.  리뷰 규칙을 간소화 • 간단한 피처는 한 사람이 바로 머지 가능 • 최소 멤버 N명이 동의하면 바로 머지 가능 • 개발 완료되면 최대한 빠르게 리뷰/머지 후 배포
55.  리뷰가 지연되는 PR이 발생 (피처 원인) • 배포가 잦다보니 배포 건 위주로만 리뷰하게 됨 • 배포 우선순위가 낮은 피처 • 테스트 기간이 많이 요구되는 피처
56.  리뷰가 지연되는 PR이 발생 (담당자 원인) • 여러 피처가 섞여 크기가 커진 PR • 작성자의 피드백이 늦은 PR • 난이도가 높거나, 한 명이 주도하는 코드
57.  오래된 리뷰는 빨리 머지 • 오래된 리뷰는 빨리 머지하자는 분위기가 형성됨 • 오래된 경우 자세한 리뷰보단 빠른 머지가 낫다 • 배포 비용도 적고, 배포 주기도 짧기 때문
58.  잦은 배포에 대한 피로감 • 매일 배포에 멤버들이 지침 • 배포 비용이 적긴 하지만, 리뷰 + 테스트 필요 • 피처 작업, 배포 준비 간의 컨텍스트 비용 큼
59.  주간 배포 방식으로 변경 • 한 주에 한 번만 배포하는 방식으로 변경 • 배포 건에 따라 화/수/목 중 선택 • 전 주 금요일까지 리뷰가 완료된 건만 배포
60.  이제 더 이상 리뷰는 병목이 아님 • 미리보기 서버로 피처 미리 공유 • 배포 일정을 컨트롤할 수 있음 • 팀원의 리뷰 속도가 빨라짐
61.  2년 동안의 개선 예외적인 상황이 발생됨 컨벤션에 대한 리뷰가 대부분 PR 규모가 커서 리뷰하기 어려움 바쁠수록 리뷰를 미루게 됨 리뷰시간이 예상보다 오래 걸림 리뷰가 병목이 됨 기획/디자인 직군의 불만 통합 테스트 때 스펙 변경이 발생함 서로에 대한 불만 리뷰 병목이 폐해 프로젝트 멤버가 늘어남 생산되는 코드만큼 리뷰 대상도 늘어남 여전히 리뷰는 지연됨 테스트 직전 리뷰가 몰림 사이즈가 큰 피처는 리뷰하기 어려움 짧아진 배포 주기 리뷰가 지연되는 PR이 발생 잦은 배포에 대한 피로감 리뷰 없이 머지할 수 없도록 제한 pre-commit 깃훅에서 린트 수행 오프라인과 온라인 리뷰를 병행 PR과 커밋 단위에 대한 합의 브랜치 미리보기 서버를 구성함 우리 팀은 코드 리뷰를 하는 팀 리뷰 팀을 구분 리뷰 데이 도입 리뷰 마스터 제도 도입 메인 피처 브랜치로 PR하도록 함 2번에 걸처 리뷰함 리뷰 규칙을 간소화 오래된 리뷰는 빨리 머지 주간 배포 방식으로 변경
62.  코드리뷰에 대한 회고
63.  리뷰가 팀에는 도움이 되었을까? • 들쭉날쭉한 코드 스타일 • 히스토리를 알 수 없는 주석 처리된 코드 • 퀄러티가 낮거나, 중복 구현된 코드 • 다른 팀원이 뭘 개발하는지 모르는 상태
64.  리뷰는 서로에게 도움이 되었을까? • 새로운 스타일 또는 접근 방법을 알게 됨 • “배울 게 많았다” • 논의 과정에서 서로 성장하는 느낌 • 이제는 안하면 뭔가 불안함
65.  리뷰가 좋긴 좋구나! • 긴급 핫픽스 코드에서 버그 발견! • 내가 짠 것 같은데 알고 보니 다른 사람이 짰음 • 인수인계 할 게 거의 없음
66.  유익하다고 느꼈던 리뷰 • 미리 발견하는 버그 • 경험의 공유 (삽질 회피, 기존 코드 히스토리) • 더 나은 제안 (로직, 변수명)
67.  조금은 불필요한 논쟁이라고 생각한 리뷰 • 취향의 차이 (if vs switch) • 애매한 수준의 제안 (변수명, 미미한 성능 개선) • 너무 먼 미래에 대한 방어 코드
68.  리뷰에 대해 공통적으로 느꼈던 스트레스 • 코드 스타일의 사소한 부분까지 강요당했을 때 • 피처도 작업하랴, 리뷰하랴, 피드백하랴 • 내일이 마감인데, 전체 구조를 변경하는 리뷰 • 내일이 테스트인데, 쌓인 리뷰가 수십 개
69.  코드 스타일은 꼭 맞춰야할까? • 한 사람이 짠 것 같은 코드 • 코드를 읽고 수정하기 편하고 • 리뷰 속도도 빨라짐 • 결국 팀의 속도가 빨라짐
70.  코드 스타일에 대한 리뷰는 필요했을까? • 코드 스타일 리뷰는 모두의 스트레스였음 • 특히 규칙이 정해져있지 않았을 때 더했음 • 시간이 아깝다고 느껴지기도 했음
71.  코드 스타일은 도구로 해결하자 • 상세한 단위까지 포맷터를 적용 • pre-commit 깃훅이 효과적 • 도구가 준비되지 않았다면 생략해도 좋을 듯
72.  그래도 이런 것들은 리뷰해야 함 • 기본 틀을 완전히 벗어나는 스타일 • 주석 처리된 코드 • 쓰이지 않는데 나중을 위해 아껴둔 코드
73.  리뷰가 왜 병목이 됐을까? • 동료의 리뷰와 동의가 있어야 머지 가능 • 동료가 리뷰를 늦게 하는 것이 문제
74.  모든 동료의 동의, 효과 있었을까? • 어렴풋하지만 대부분의 코드를 알고 있었음 • 자연스럽게 동료가 어떤 작업을 하는지 알게 됨 • 효과는 좋음. 인원이 많아지며 병목의 원인이 됨 • 최소 2~3명의 리뷰는 유지하는 게 좋겠음
75.  어떻게 하면 빨리 리뷰하게 할까? • 전체 동의 + 리뷰 마스터 제도 (스토리웹) • 리뷰어 2명 지정 + 위임 (아틀라시안) • 머지 여부를 본인이 결정 (스토리 iOS)
 76.  책임자를 지정하는 것은 효과적 • 리뷰 마스터든 리뷰어든 담당자를 지정 • 개인의 책임 > 공동의 책임
77.  몇 명의 팀일 때 리뷰가 가장 잘 될까? • 2명: 피드백 빠르지만 논의 상대 부족 • 3~5명: 전체 동의 조건으로 효과적이었음 • 6명~: 의견/논의도 많음. 결과 대비 비효율적
78.  시간이 갈수록 리뷰속도가 빨라짐 • 일관성 있는 코드 스타일 • 각자 중요하다고 생각하는 포인트 위주로 리뷰 • 배포 주기가 짧아 쉽게 수정 배포 가능한 배경
79.  개발 속도 느리니까 코드리뷰 하지마! • 어떻게 보면 커뮤니케이션의 문제인 듯 • 타직군은 코드리뷰에 관심 없음 • 중간 과정을 공유하고 좋은 결과물을 보여주자 • 리뷰는 개발자의 권리
80.  우리가 리뷰를 유지할 수 있었던 이유 • 초기부터 모두의 동의 하에 자율적으로 도입 • 코드 리뷰는 당연하다라는 문화의 정착 • 문제의 인식과 지속적인 개선 노력 • 정기적인 개발 미팅
81.  특히, 정기적인 개발 미팅 좋았음 • 매주 정해진 시간에 자유 주제로 논의 • 이터레이션 종료 후 회고 • 리뷰 정책 / 개선에 대한 논의할 수 있는 기회
82.  우리가 리뷰를 유지할 수 있었던 환경 • 엔터프라이즈 깃헙 • 모두 같은 언어로 같은 서비스를 담당 • 지속적으로 한 서비스를 담당 • 수평적 문화 (영어 이름)
83.  반면, 예전 좋지 않았던 리뷰 경험 • 팀 내에서 코드 리뷰 진행 • 자바스크립트 개발자가 모여있는 기능 조직 • 각자 다른 프로젝트에 투입 • 자율적으로 만들어진 분위기가 아니었음
84.  왜 그랬을까? 주로 오프라인 코드 리뷰 • 리뷰 미팅에서 프로젝터로 공유 • 리뷰 미팅은 분위기 영향을 많이 받음 • 과열된 논쟁이나 귀차니즘의 전파
85.  왜 그랬을까? 도구의 부실함 • SVN + 마땅한 리뷰 도구 없었음 • 메모장이나 에디터에 주석으로 달아 전달 • 별도 리뷰 도구를 도입했지만 잘 연동되지 않음
86.  왜 그랬을까? 서로 다른 업무 • 개발하는 언어는 같았지만 • 담당하는 서비스가 모두 다름 • 리뷰 범위의 한계 (스타일이나 일반적인 로직)
87.  왜 그랬을까? 주니어의 코드가 리뷰 대상 • 주로 주니어의 코드가 리뷰 대상 • 시니어가 주니어의 코드를 고쳐주는 일방향 리뷰 • 시니어들에겐 큰 도움이 되지 않음
88.  왜 그랬을까? 자리잡지 못한 문화 • 코드 리뷰가 잘 워킹하지 않는 걸 모두 알고 있음 • ‘뭘 코드리뷰를 해~’라는 분위기 • ‘팀별로 코드리뷰를 하라’는 상위 조직의 강제성
89.  내가 느낀 코드리뷰의 장점 (개인) • 자연스럽게 내 코드를 퇴고하게 됐다 • 다른 사람의 코드를 읽는 시간이 많아졌다 • ‘왜 이렇게 했을까’ 생각하고 • ‘왜 이렇게 했는지’ 설명하는 시간이 많아졌다
90.  내가 느낀 코드리뷰의 장점 (팀) • 리뷰에서 잡아내는 버그가 생각보다 많았다 • 유지보수, 인수인계 비용이 줄어들었다 • 팀에 자연스러운 토론 분위기가 형성됐다 • 동반 성장하는 느낌이 들었다
91.  내가 생각하는 코드리뷰의 단점 • 여전히 리뷰가 병목이 될 수 있다 • 리뷰 문화 정착까지의 비용이 크다 • 가끔은 리뷰가 생산 의욕을 꺾을 때도 있다
92.  팀에 코드리뷰 도입하기
93.  팀에 리뷰를 도입할 때 꼭 챙길 것 • 깃헙이나 그에 준하는 온라인 리뷰 도구 • 코드리뷰를 하고 싶어하는 마음 맞는 동료 • 개발자들과 자주 이야기할 수 있는 환경
94.  흔히 우리가 맞이하는 상황 • 코드리뷰를 도입해보고 싶지만 • 다른 팀원들의 적극적인 호응이 없고 • 어떻게 시작해야할 지 감이 잡히지 않는다
95.  새로 시작하는 조직에 리뷰를 도입한다면 • 모든 멤버의 자율적 동의로 시작하고, • 최대한 강제성을 적용한 규칙으로 시작 • 규칙은 도구를 사용해 제한
96.  시작할 때는 이렇게 해보면 어떨까? • develop,master push 제한 / 모두 PR로 • 모든 멤버의 동의 • 코드 스타일 체크는 자동화 (깃훅 등)
97.  기존 조직에 리뷰를 도입한다면 • 나와 마음이 맞는 동료를 찾아 소규모로 시작 • 도구를 적극적으로 활용 • 기존에 일하던 방식에 자연스럽게 적용될 수 있게 • 다른 멤버가 거부감을 갖지 않도록 천천히 도입
98.  근데 좀 해보려고 하면, • 다른 멤버는 시큰둥하다. 나만 하고 싶나… • 자꾸 하자고 하려니 귀찮고 미안하다… • 그냥도 이미 바쁜데 오바 아닌가…
99.  좋은 거 아니까, 극뽁!
100.  코드리뷰는 문화 • 기존의 습관을 단번에 바꾸기 어려움 • 억지로 바꾸려고 하면 더 어려움 • 여유와 시간을 갖고 천천히 • 정답은 없음. 우리 팀에 맞는 방식으로.
101.  작은 경험의 반복으로 익숙해지도록 • 내 코드를 먼저 리뷰하도록 시도 • 처음엔 리뷰하기 쉽도록 PR은 가능한 작게 • Pull Request - 리뷰 - 머지의 경험 반복 • (선택) 리뷰어를 지정해서 부탁
102.  리뷰에 어떻게 반응하면 좋을까? • 피드백! 피드백! • 반영 여부는 본인이 결정하는 것이 좋은 듯 • 코드는 내가 아니고, 그저 내가 작성한 코드일 뿐
103.  어떻게 리뷰하면 좋을까? • 부드럽고 젠틀하게 • 궁금한 건 의도를 물어보는 식으로 접근 • 이견이 있다면 구체적인 방법을 제시 • 마음이 불편하더라도 적극적으로 리뷰
104.  리뷰 문화를 잘 유지하려면 • 적극적으로 리뷰하고 잘 피드백하자 • 코드 스타일 리뷰는 말 대신 도구로 처리하자 • 서비스와 코드, 리뷰에 대해 자주 이야기하고 • 리뷰가 병목이 되지 않게 개선하자
105.  Q & A
 106.  (부록) 코드 리뷰 예
107.  오타
108.  가독성
109.  네이밍
110.  성능