얼마 전, 렘(Realm)의 기술 블로그에 올라온 코드리뷰, Github로 바로 적용하기 - Realm에서의 코드리뷰 소개라는 글이 많이 회자되었죠.
카카오는 어떻게 하고 있을까~ 궁금해서 수소문을 했더니, 사내에서도 깃헙 잘 쓰기로 소문 난 카카오스토리 웹 클라이언트팀은 코드 리뷰도 잘하더군요 @..@
그들의 코드 리뷰 경험을 “날 것” 그대로 공유합니다. 우리끼리 보려고 쓴 글이라, 표현은 조금 거칠지만 더 쉽게 와 닿네요.
웹 클라이언트 개발팀 내에선 코드 리뷰가 굉장히 활성화되어 있는데요, 얼마 전에 문득, 코드 리뷰를 도입하면서 경험했던 내용을 공유해보면 좋을 것 같단 생각이 들어서, 짬짬이 예전 기억을 되살리면서 노트에 정리했습니다.
지금은 리뷰 문화가 정착돼서 그저 당연한 거라고 생각하고 있었는데, 돌이켜보면 도입 과정에서 탈도 많았고 여러 이슈를 극복하려고 멤버들 모두 고생했던 게 떠오르더군요. 다른 팀이 어떻게 일하고 있는 지 모르고, 작은 조직 내에서의 얕은 경험일 수도 있겠지만, 개인적으론 코드 리뷰를 도입하고 유지하고 있는 게 굉장히 값진 경험이라고 생각하고 있습니다.
처음엔 그냥 웹 클라이언트 멤버들끼리만 돌려보려고 했는데, 아직 코드 리뷰를 도입하지 않은 팀들도 있는 것 같고, 정리하다 보니 다른 분들이 보셔도 좋은 내용일 것 같아 조심스레 공유해봅니다.
거창하게 “이렇게 저렇게 했다”라고 공유하는 것보단, 쓱~ 부담없는 메모 형식으로 공유합니다. 좀 러프해도 이해해주세요^^;
처음엔 셋이서 시작
- 2013년 5월부터 스토리 웹 프로젝트에 도입
- 엔터프라이즈 깃헙!
- 깃 플로우 기반 + Pull Request 활용
- 모든 코드 리뷰 원칙
리뷰 방법
- 피처 브래치에서 작업 후 Pull Request,
- 디벨롭 머지 시 리뷰 요청
- 모든 멤버가 동의했을 때 디벨롭에 머지
리뷰 없이는 머지되지 않도록 제한
- master, develop 브랜치로는 바로 푸시하지 못하도록 push 깃훅 추가
- 프로젝트를 클론할 때, 깃훅을 반드시 설치하도록 강제함
가장 많이 발견되는 리뷰
- 컨벤션
- 들여쓰기, 괄호, 공백, 캐멀케이스, 언더스코어…
컨벤션 리뷰로부터 벗어나는 법을 고민하기 시작
- 컨벤션 리뷰는 감정 상하고 불필요하다고 느꼈음
- 도구로 해결할 수 있는 법을 찾다가, commit 깃훅에 린트 작업을 추가함
- 컨벤션 리뷰가 눈에 띄게 줄었음
commit 깃훅에 린트 검사 적용하기
- 해당 커밋에서 변경이 발생한 파일에 대해서만 린트 검사를 수행함
- 한 번에 모든 코드를 수정하지 않아도 되어서 거부감이 적었음
- 최초로 내가 작성한 코드가 아니더라도 내가 그 파일을 수정하면 린트 오류를 수정해야 함
- 점진적으로 코드가 정리되었고, 얼마 지나지 않아 프로젝트 전체의 린트 오류가 0%가 됨
유익하다고 느꼈던 리뷰들
- 미리 발견하는 버그
- 기존 코드의 히스토리
- 삽질 회피 코드의 공유
- 더 나은 로직의 제안
- 더 나은 변수명 제안
조금은 불필요한 논쟁이라고 생각했던 리뷰들
- 취향의 차이 (if vs switch)
- 바꾸기도 뭐하고 안 바꾸기도 뭐한 애매한 수준의 변수명 제안
- 아주 미묘한 성능 개선 제안
- 너무 먼 미래에 대한 방어 코드
리뷰이는 리뷰에 어떻게 반응하면 좋을까?
- 리뷰어의 의견은 잘 청취하고,
- 의도를 잘 설명하되,
- 결정은 본인이 하는 게 좋다고 생각함
- 리뷰어가 그 코드가 정 마음에 들지 않았다면, 다음 기회에 직접 수정하는 걸로
프로젝트 초기, 기초 구조에 대한 리뷰
- 프레임워크를 포함해 커밋의 단위가 컸음
- 커밋 단위가 커서 리뷰할 코드가 너무 많았음
- 코드 사이즈가 커서 전체 흐름을 파악하기도 어려움
- 먼저 오프라인 미팅으로 리뷰 진행 (전반적인 의도 설명, 코드에 대한 자세한 설명은 생략)
- 이후에 온라인 코드 리뷰
- 리뷰 단계에서 전체 코드를 버린 적도 있음
리뷰가 병목이 되었다
- 알파 서버가 develop 브랜치를 바라봄
- 테스트는 알파 서버 배포 이후부터 시작
- 리뷰가 길어지다보니 develop으로 제 때 머지되지 못해, 결국 알파 서버에도 배포되지 못함
- 기획, 디자이너 등 다른 직군이 해당 건을 테스트하기 어려움
바쁘면 바쁠수록, 리뷰 시간도 길어졌다
- 각자 피처 작업하기에 바쁘다보니, 리뷰를 미루게 됨
- 리뷰는 밀리지만 새 리뷰 건은 더 늘어남
- 리뷰 대기 시간이 길어져, 태스크 관리 도구에서 ‘개발 리뷰 중’ 단계를 추가해야 할 정도
- 나중엔 리뷰가 10개 이상 쌓여있는 경우도 있음
- 큰 피처인 경우 리뷰에 1시간 이상 걸릴 때도 있고,
- 쌓인 코드 리뷰에 하루를 다 쓰는 경우가 많아짐
병목이 새로운 병목을 만들었다
- develop으로의 머지가 늦어지니 테스트도 늦어짐
- 각 피처가 한꺼번에 머지되다보니, 미리 할 수 있었던 피처별 테스트도 한꺼번에 진행하게 됨
- 여기서의 피처 테스트는, 기획자와 디자이너의 의도가 잘 반영되었는지, 실제 서비스에 적용했을 때 무리가 없을지 확인하는 목적의 ‘사용성 테스트’였음
- 머지가 완료되고 알파에 배포된 후에 기획이 수정되는 경우가 빈번히 발생됨
- 머지 전에는 기획자나 디자이너가 구현된 피처를 확인할 수 있는 방법이 제한적임
- 개발자 자리에서 확인하거나 로컬에 붙어 확인할 수는 있었는데, 작업량과 피처가 많다보니 원활하지 못했음
- 미리 확인하기 쉽지 않아, 자연스럽게 미뤄지다가 결국 통합 테스트 때에야 발견하게 됨
- 이 때 피로도가 가장 컸던 것 같음
- 개발자는 개발자대로 불만: 일정이 얼마 남지 않았는데, 이제와서 피처에 대한 근본적인 수정이라니
- 기획이나 디자인도 불만: 개발은 다 됐다고 했는데, 왜 미리 볼 수도 없고 이제서야 확인이 가능한 건지
- 더불어, 야근이 많아지고 의욕이 떨어짐
- 빠듯한 일정 때문이기도 했지만, 프로젝트의 흐름이 매끄럽지 못했음
- 원인은 여러 가지였겠지만, 리뷰의 병목 또한 그중 하나였을 거라 생각함
개발을 완료했다고 했는데, 확인할 수 없음
- 리뷰가 길어져서 타직군이 develop 머지 전에 해당 피처를 확인하기 어려움
- 개발자 자리에서 볼 수 있지만, 개발자가 자리에 없는 경우가 있음
- 로컬 서버를 바라보도록 했지만 한계가 있음
- 작업 과정에서 로컬 서버가 계속 갱신됨
- 이미 다른 피처를 작업하고 있는 경우가 많음
- develop 머지 없이 현재 작업 중인 브랜치를 바로 공유할 수 있는 방법이 없을까?
브랜치 미리보기 서버
- 깃헙의 gh-pages에서 아이디어를 얻음
- 공유하고자 하는 브랜치를 특정 패턴의 이름으로 푸시하면, 해당 기능의 스냅샷이 배포되도록 서버를 구성함
- 예를 들어, 개발자는 작업하던
feature/XXX브랜치를preview/XXX로 푸시하면 됨 - 푸시 후엔
XXX.story.kakao.com같은 URL로 공유할 수 있음
미리보기 서버를 도입한 후
- 이거 정말 효과 와따임!
- 기획자나 디자이너에게 피처를 빠르게 공유할 수 있게 되었음
- 더불어, 아이디어의 프로토타입 버전을 작성해 공유하는 용도로 활발하게 사용됨
- 다음 프로젝트에도 꼭 도입하고 싶음!
새 멤버가 들어오면 잘 적응할 수 있을까?
- 이제는 ‘모든 코드의 리뷰는 당연하다’라는 분위기가 만들어진 상태
- 새 멤버가 들어오면 이 리뷰 문화를 받아들일 지 걱정
- 새 멤버가 리뷰에 거부감이 있으면 어떡하지?
- 걱정하다가, 알아서 잘 하겠지… 라고 결론… -_-ㅎㅎ
프로젝트 멤버가 늘어남
- 처음엔 3명이었는데, 8명까지 늘어남
- 새 멤버들 모두 리뷰 문화를 잘 받아들여줌
- 대부분의 멤버가, 회고 미팅 때 리뷰가 스트레스였음을 얘기해주긴 했음
새 조직에서 리뷰 문화를 만들어야 한다면…?
- 리뷰는 당연한 과정이라고 느껴지도록 만들어야 할 듯
- 모든 멤버가 리뷰 문화를 받아들여야 함
- 코드는 내가 아니고, 그냥 내가 작성한 코드일 뿐, 상처받지 말아야 함
멤버가 늘어나니 생산되는 코드도 늘어나고, 해야하는 리뷰도 늘어남
- 생산되는 코드의 양이 늘어남
- 이제 모든 코드를 보기엔 시간이 부족한 상황
- 그렇다고 모든 팀원이 리뷰하기를 기다릴 시간적 여유도 없음
- 팀을 나눠서 반반씩 리뷰를 진행하기로 함
- 마침 조직 개편되면서 자연스럽게 그룹이 나뉘어졌지만, 이런 상황이라면 내부적으로 리뷰팀을 나눠도 좋을 듯
- 필요한 경우, 리뷰어를 지정함 (예: 해당 리뷰어가 전문적으로 잘 알거나, 대부분의 코드를 작성한 경우)
- 프로젝트 초반엔 모든 코드의 리뷰를 목표로 했지만, 이제는 리뷰하지 못하고 머지되는 코드들이 많음
리뷰 마스터
- 일정에 쫓길수록, 피처 작업 때문에 리뷰가 밀리게 됨
- 테스트 해야할 날짜가 다가오지 않으면 아무도 리뷰하지 않는 상황
- 결국 테스트 전날에 몰아서 리뷰하고 수정하게 되는 문제가 발생
- 주기적으로 리뷰를 독려하고 머지를 담당하는 ‘리뷰 마스터’ 제도 도입
- 리뷰 마스터는 한 주 또는 특정 주기로 쌓여있는 PR을 빨리 리뷰하도록 처리하고,
- 개인 판단 하에 빠르게 머지 가능한 것은 머지함
- 조금은 장난 삼아 도입했는데, 의외로 굉장히 효과적이었음
스펙이 큰 피처인 경우엔,
- 피처 단위가 커서 한 번에 리뷰하기엔 코드량이 너무 많음
- 한 피처를 여러 명이 작업하게 됨
- 해당 피처의 큰 줄기인 feature/A 를 따고,
- 하위 태스크를 feature/A-1, feature/A-2 로 땀
- 각 피처의 하위 태스크가 끝나면 상위 피처로 Pull Request를 넣음 (예: feature/A-1 –> feature/A)
- 각 피처 담당자끼리 1차 리뷰 진행
- 이 때엔, 큼직한 구조나 로직에 대해서만 러프하고 빠르게 리뷰함
- 물론, 피처 담당자가 아니더라도 리뷰하고 싶을 때 자유롭게 남길 수 있음
- feature/A 를 develop 에 머지할 때 2차 리뷰
- 리뷰가 끝에 몰리지 않고, 두 번째 리뷰부턴 확실히 속도가 빨랐음
- 리뷰를 단계별로 나눈 것은 좋은 결정이었다고 생각함
적극적인, 잦은 배포
- 예전엔 이터레이션 별로 태스크를 묶어서 배포함
- 프로젝트가 운영 모드로 들어가고 안정기에 접어들면서, 배포 환경도 개선되었고 비용도 줄어들음
- 이제는 계획보단 실행에 포커스를 맞춰서, 구현되는 태스크를 가능한 빨리 배포하기로 결정함
- 팀별로 나눠 리뷰하기로 했던 룰이 여전히 존재하긴 하지만,
- 리뷰가 가능한 멤버 N명이 동의하면 바로 머지 가능
- 피처 사이즈에 따라 다르지만, 개발이 완료되면 최대한 빠르게 머지, 테스트 후 배포함
유연해진 리뷰
- 간단한 건은 한 사람도 바로 머지 가능
- 마크업 수정, 코드 변경 단위가 적고 큰 영향이 없는 건
- PR이 올라온 지 오래된 리뷰는, 빨리 머지하자라는 분위기가 만들어짐
- 개인적으로도, 이런 건은 큰 문제가 없다면 자세한 리뷰보다는 빠른 머지가 더 의미있다고 생각함
- 좋게 보면 유연해졌지만, 나쁘게 보면 좀 대충 대충 느낌도 있음
- 머지 사이클도 빨라져서, 리뷰가 병목이 되는 상황은 조금 줄어들었음
- 복합적인데, 배포 비용도 적고 주기도 짧아서, 쉽게 수정해서 배포할 수 있다는 생각이 근저에 깔려있기 때문인 듯도 함
한정적인 리뷰 시간, 각자의 리뷰 포인트가 있음
- 배포도 잦아지고 처리해야할 작업도 많아지면서, 리뷰 시간이 갈수록 줄어들음
- 코드를 다 볼 수 없으니, 중요하다고 생각되는 포인트들만 리뷰하게 됨
- 변수명이나 컨벤션 리뷰는 거의 안하게 됨
- 나는 주로 아래 항목 위주로만 리뷰함
- 예외 처리가 잘 되어 있는지 (예: 컨텐츠가 없다거나, 유효하지 않은 파라미터라거나)
- 좀 더 간결하게 작성할 수 있는 방법이 있는지
- 버그가 발생할 가능성이 있는 코드인지
- 병목이 될 가능성이 있는 코드인지
- 빌드/배포 관점에서 (이건 내가 주로 작업했던 부분이라)
- 다른 멤버들도 따로 얘기하진 않았지만, 비슷하게 각자의 포인트를 잡고 리뷰하는 듯 함
- 예) 누구는 성능 위주로, 누구는 프레임워크 위주로, 누구는 크로스브라우징 위주로, 누구는 모바일 위주로 등등
리뷰가 늦어지는 PR들은?
- 코드의 양이 너무 많은 경우
- 우선순위가 낮은 피처
- 테스트가 많이 필요해보이는 피처
- 좀 아이러니한데, 매일 배포할 정도로 배포가 잦아지다보니, 배포 건 위주로만 리뷰하게 됨
- 코드 작성자의 피드백이 늦은 Pull Request
- 난이도가 높거나, 한 명이 주도하는 코드
- 이 경우, 다른 리뷰어들이 리뷰하기 어려워 미뤄두다가, 시간이 흐른 뒤에 이제는 머지해야겠다는 분위기로 머지되는 경우가 많았음
- 그러고보면, 진작에 머지해도 됐던 것 같기도 하고… -_-
우리 팀의 코드 리뷰가 여전히 잘 워킹하고 있는 이유…
- 코드 리뷰를 모두의 동의 하에 자율적으로 도입함
- 훌륭한 도구 (깃헙은 정말 짱인 듯… 이제는 깃 없이 작업하는 건 꿈도 못 꿀 지경…)
- 코드 리뷰는 당연하다라는 문화가 정착됨
- 모두 같은 프로젝트를 하고 있음
- 모두 같은 코드를 사용함
반면, 예전에 경험했던 좋지 않았던 리뷰 경험
- 이전에도 코드 리뷰를 했었지만, 원활하지는 못했음
- 팀 내에서 코드 리뷰를 진행함
- 우리 팀은 자바스크립트 개발자가 모여있는 기능 조직이었고, 대부분 각자 다른 프로젝트에 투입되어 있었음
- 자율적으로 코드 리뷰를 해보자는 분위기는 아니었음
- 주로 오프라인 코드 리뷰
- 작성자가 프로젝터로 공유하고 싶은 (또는 문제가 되는) 코드를 공유
- 리뷰 미팅 참석자들이 프로젝터를 보고 리뷰함
- 프로젝터로 보기엔 눈도 아프고, 시간 상 특정 코드를 유심히 보기도 어려움
- 리뷰 미팅이 분위기에 굉장히 영향을 많이 받음
- 논쟁이 붙어서 분위기가 무거워지는 때도 있었지만,
- 귀차니즘의 전파가 더 무서움. 이럴 땐 이걸 왜하고 있나 하는 생각이 들 때도 있었음
- 도구의 부실함
- SVN을 쓰고 있었고, 마땅한 코드 리뷰 도구가 없었음
- 해당 코드를 복사해서 메모장으로 리뷰를 전달하기도 했고, 에디터에 주석으로 달기도 했음
- 나중에 다른 리뷰 도구가 도입되긴 했지만, SVN과의 연동이 매끄럽지 않았음
- 서로 다른 업무
- 개발하는 언어는 모두 같았지만,
- 담당하는 서비스가 모두 달랐음
- 리뷰어가 전반적인 로직이나 피처를 이해하지 못했기 때문에, 컨벤션이나 일반적인 로직에 대한 리뷰가 대부분이었음
- 주니어의 코드가 리뷰 대상
- 주로, 주니어의 코드가 리뷰 대상이었음
- 시니어가 주니어의 코드를 일방적으로 가이드해주는 방식이 대부분이었음
- 주니어에겐 도움이 되긴 하는데, 나머지 시니어들에겐 큰 도움이 되지 않았던 듯
- 자리잡지 못한 문화
- 코드 리뷰가 잘 워킹하지 않는다는 걸 사람들도 알고 있었음
- ‘뭘 코드 리뷰를 해~’라는 분위기가 은근히 깔려 있었음
- ‘팀별로 코드 리뷰를 하라’는 상위 조직의 강제성도 있었음
내가 생각하는 코드 리뷰의 장점
- 다른 사람의 코드를 읽는 시간이 많아졌다
- 다른 사람의 코드를 보고 많이 배우게 된다
- 리뷰 단계에서 잡아내는 버그가 생각보다 많다
내가 생각하는 코드 리뷰의 단점
- 여전히 리뷰가 병목 지점이긴 한 것 같다
- 이 병목이 나중에 발생하는 버그의 비용보단 작을 것 같단 생각이 들긴 한다
- 가끔은 리뷰가 생산 의욕을 꺾을 때도 있다
- 누군가 열혈 코딩으로 엄청난 프로덕트를 생산해낼 수 있는데, 리뷰 단계에서 엄청 까였다고 생각해보자
팀에 리뷰를 도입할 때 꼭 챙길 것
- 깃헙… 깃헙…
- 컨벤션 리뷰는 말 대신 도구로 처리하자. 깃훅 강추
- 가능하면, 젠틀하고 완곡하게 표현하자.
- 코드는 내가 아니라 코드일 뿐이지만, 나는 여전히 감정적인 사람이니까…
- 리뷰어는 의견을 제시할 수 있지만, 수정에 대한 결정은 작성자가 하도록 한다
에피소드
- 커멘트가 200개 넘게 달린 PR도 여러 개 있음 (is:pr is:pr comments:200..1000)
- 리뷰 단계에서 머지되지 못하고 닫힌 Pull Request도 많음 (is:pr is:closed is:unmerged)
- 사내에서 깃헙을 가장 활발하게 사용하고 있는 프로젝트로 선정됨!
어떠신가요? 두서없이 나열했지만 입에 착착감기고 눈에 속속 들어오지 않나요?
카카오에서는 개발 문화의 일부로 코드 리뷰를 정착시키기 위해 Crucible 같은 전용 소프트웨어도 도입하고, 의무화하는 등 여러가지 방법을 동원했지만 아직도 온전히 자리잡지 못했습니다.
여기에서 공유한 경험이 모든 회사나 조직에 그대로 적용되지는 않겠지만, 시행착오를 줄이고 더 좋은 개발 문화를 만들어가는데 보탬이 되길바랍니다. 앞으로도 완벽하지는 않지만, 실제로 돌아가는 카카오의 개발 문화를 지속적으로 소개하겠습니다.