코드 리뷰... 어떻게 하세요?

페어 프로그래밍 하면 코드리뷰를 실시간으로 하는 것이 되어서 따로 코드리뷰 안합니다. 다른 개발자가 새 change set 커밋하면 가끔 diff 보기도 하고 문제를 발견하기도 합니다(다른 이들도 비슷한 일 합니다).

제 세번째 직장(현재 다섯인가 여섯번째 직장입니다)에서는 코드 라인에 따라 몇명의 시니어가 참석하고 시간은 얼마를 해야된다는 가이드 라인이 있었습니다(외국 통신회사였습니다). 프리젠테이션 하는 스타일로 아이디어 설명, 함수별 설명등을 하고 나면 각자 질문하고 커멘트 남기고 시니어들이 ok하면 다음 릴리즈에 반영되었었죠. 대개의 경우 20분 전후로 끝나고 한 30분 차마시면서 잡담하고 돌아와서 다시 일했었죠(갑자기 그때 같이 일했던 외국 동료들 얼굴과 이름이 생각나네요. 벌써 8년전 이야기네요!).

참, change set 커밋은 버전컨트롤 정책에 따라 다르겠지만, 저희 경우는 리뷰 없이도 커밋 합니다. 단지 stable branch에 merge하지 않을 뿐이죠. 즉, 변경이 매우 중요하면 모두에게 change set ID를 알려서 한번 리뷰해줄 것을 요구하기도 합니다. 그 후, ok되면 stable 에 merge, 아니면 다시 수정한 후 다시 리뷰 요청, etc. 저희는 mercurial (cvs에서 mercurial로 전환한지 한 7-8개월 되는 것 같네요) 사용하는데, change set은 로컬 혹은 main repo에 백업 개념으로 항상 duplication 만들어 놉니다.

아참, 테스트 케이스 모두 통과해야 stable에 merge 합니다. 테스트 케이스 한 500여개 되나? 요즘 테스트 프레임워크 통합일을 좀 손보고 있는데, 골치아프네요.

개인적으로는 페어 프로그래밍이 명시적 리뷰보다 낫다고 봅니다.

일단 회사라면 각 구성원의 능력이 비슷했을때를 가정하고..
1. 대규모 프로젝트가 아니라 소수라면 그냥 커밋 (대규모 소규모는 정하기 나름)
1. 유닛 테스트 병행
1. 주기적으로 코드 리뷰
1. 커밋되는 량이 일정수준 이상이면 리뷰 주기 단축시키고
1. 문제가 발생하면 테스트 케이스 추가 및 리뷰

원래 버전 콘트롤시스템이 나온것이 개발을 위한 임시저장소의 성격도 가지고 있는것인데, 완벽하지 않으면 커밋하지 않게 해버리면 그 효용성을 절반이상
잃어버리게 되죠. 그런 경우라면 차라리 cvs/svn 말고 git/bazzar 계열의 분산형으로 옮기는게 낫겠고요.

개발 (혹은 초기개발단계) 성향의 프로젝트인지, 이미 성숙하고 안정된 프로젝트에 대한 개발인지에 대해서도 조금씩 다를 것이고 등등등

기본적으로 Perforce를 사용하고 그 위에 자체 플러그인을 이용해서 review를 쉽게 할수 있도록 도와줍니다. 현재 pending중인 changelist들의 파일들의 diff를 묶어서 압축한 후, 공유폴더에 자동으로 복사가 되고, code review때는 그 링크만 보냅니다. reviewer가 그 링크를 클릭하면 자체 플러그인은 하나씩 merge된 파일과 merge전의 file을 양쪽에 보여주고 (windiff 같은 화면) 리뷰를 하게 됩니다. 하지만 code review가 꼭 필요하기 때문에 code review요청 이멜이 너무 많아짐에 따라 분야별로 주 reviewer를 따로 두고 있습니다. 하지만 이렇게 해도 나중에 어떤 리뷰어가 뭐라고 했는지 효과적으로 기록을 남겨놓을수 없기 때문에 현재는 Google의 Mondrian의 영향을 받아 비슷한 시스템을 개발중입니다.

저희도 코드 리뷰를 안 해서 새로운 정보를 드릴 수는 없지만, 지금 하시는 방법과 크게 다르지 않으면서 툴의 도움을 받을 수 있는 괜찮은 솔루션이 있습니다.

설치 방법도 어렵지 않더라구요.

reviewboard입니다.
http://www.review-board.org/
----------------------------
May the F/OSS be with you..

우리 회사에 코드 리뷰하는 부서가 있을라나 모르겠습니다.
일단 저희 부서는 안합니다만..

kldp.net에 nFORGE 시스템 적용되길 기다리고 있습니다.
제 개인 프로젝트나 어셈러브 모임 프로젝트를 준비하고 있거든요.
화이팅~

----
섬기며 사랑하면 더 행복해집니다.
개인 홈페이지가 생겼습니다 http://caoskernel.org
어셈러브를 개편중입니다 http://www.asmlove.co.kr

보통은 한번 리뷰를 진행할때 7~8명 이내의 인원이 1시간을 넘지 않도록 해야한다고 나와있습니다.
이렇게 했을 경우(NASA에서 실험한 결과) 약 90% 이상의 bug를 사전에 찾을 수 있다고 합니다.
이렇게 좋은 것이 실제 현업에서 많이 사용되고 있지 않는 이유는
첫째로 비용이 만만치 않습니다.
둘째로 제대로된 리뷰를 하지 않을 경우 결과가 좋지 않습니다.

일단 코드리뷰를 진행하기로 했다면 두번째 문제가 생길 가능성이 큽니다.
어떻게 코드리뷰의 결과를 좋게 만들것인가를 고민해야합니다. 그렇지 않으면 시간(비용)은 시간대로 허비하게 되고, 단지 우리는 코드리뷰를 했다라는 허울뿐인 명예만 남습니다.
윗분들 중에 코드리뷰 후 테스트를 진행한 후 문제가 없으면 commit을 한다고 하셨는데 이것 또한 생각해 보아야할 문제 입니다.
코드리뷰를 테스팅과 연관을 가지고 갈것인지 아니면 별개로 가지고 갈 것인지도 결정해야 합니다.
테스터가 따로 있는 경우는 별개의 프로세스로 가지고 가는것을 추천합니다만 각 회사의 사정에 맞게 프로세스를 구성해야합니다.
보통 코드리뷰는 개발자가 해야하는 몫이고, 테스트는 테스터가 해야할 몫이기 때문입니다.(단위 테스트는 물로 개발자가 합니다.)
분명히 말씀드리는 부분은 코드리뷰와 테스트가 커버하는 bug의 범위가 다르기 때문에 단지 테스트를 통해서 발견된 버그가 모든 버그라고 생각하면 안됩니다.

요새는 보통 코드리뷰를 툴로 돌리는 경우가 많은데(비용문제상) 이것도 장단점이 있으므로 고민을 해보셔야 할 듯 합니다.

저는 조금 다른 관점에서 의견을 말씀드리자면...

-> 1. 좋은 코드리뷰를 위해서는 좋은 기획과 설계가 밑바탕되어야 한다.
-> 2. 1번 항목에 대해서 팀원 및 코드리뷰에 참석하는 개발자들이 정보를 공유하고 있어야 하고, 어느 정도의 사전지식이 필요하다.
-> 3. 코드리뷰에서 syntax error, memory leak, array boudary, side effect.. 등과 같은 문제를 찾을려고 하면 안된다. 코드리뷰는 unit test 와 integrity test 사이에 하는 것이 바람직하다.
-> 4. 코드리뷰의 양 또는 모듈은 제한적이어야 한다.
-> 5. 기타등등

<- 1. 기획과 설계없이 개발자 혼자 작성한 코드를 리뷰를 하게되면 코드작성자외에는 아무도 관심을 갖지 않을 겁니다.
<- 2. 코드리뷰할 코드에 대한 정보공유와 사전지식이 없다면 회의시간에 그 코드의 오류와 버그를 찾을려고 생트집을 잡던지 그냥 빨리 끝나기만을 바랄 뿐입니다. 혹은 메일내용을 by pass 할지도 모릅니다.
<- 3. 최소한 코드리뷰시간이 syntax error와 debuging 시간이 되지는 말아야 합니다. 유닛테스트는 이제 기본입니다.
<- 4. 빔프로젝트로 생전 처음보는 수천라인의 소스코드는 그리 눈에 잘 들어오지 않습니다. 인간의 뇌 buffer 는 8K 가 안될지 모릅니다.
<- 5. 개발자들의 귀차니즘과 빡빡한 일정은 어떤 좋은 인터페이스, 어떤 좋은 소프트웨어라도 by pass 마스터키가 된다.

저희는 Code Collaborator 라는 툴을 도입해서 4달정도 사용중입니다.

진행하다 보니 나름 우리 팀의 환경에 맞는 프로세스가 구축이 되었는데, 다음과 같습니다.

1. 작성자는 소스코드를 수정후 체크인 하기 바로 직전에 코드 리뷰 등록을 한다.
2. 이때 다른 팀원중 1명을 Reviewer로, 나머지 팀원들은 Observer로 등록한다.
(Observer도 리뷰 내용을 볼 수 있고, 커멘트 할 수 있지만, Reviewer만이 완료처리할수 있다)
3. 작성자는 코드 diff를 올린 다음 중요한 내용들은 스스로 설명하는 커멘트를 추가한다.
4. 작성자는 코드 리뷰 등록을 완료하면 체크인(커밋)한다.
5. 리뷰어는 하루 1시간 정도를 할애하여 리뷰한다.
6. Observer는 반드시 리뷰해야하는 의무는 없다.

이정도 이군요.
코드 리뷰가 완료되어야만 체크인하는 팀들도 있다고 하고, 2명이상 리뷰어를 지정하는 팀들도 있다고 하는데, 우리 팀한테는 좀 헤비하다고 느껴져서 위와 같이 진행하고 있습니다.

좀 더 자세한 내용은 블로그 참조 ( www.codereview.co.kr )

페어 프로그래밍을 시도한적이 있으나 실패했었구요, 페어프로그래밍을 한다면 굳이 코드리뷰를 따로 할 필요는 없을거라 생각됩니다.
리뷰어가 2명을 넘어가면 별로 추가이득이 없다는 생각입니다.
그리고, 유닛테스트/정적분석툴 등과 코드리뷰는 서로 보완적이라고 생각됩니다. 코드리뷰의 가장 큰 미덕은 에러의 검출보다 오히려
지식의 공유(또는 교육)와 이해하기 쉬운 코드 만들기라고 생각됩니다.

리뷰 당일에 갑과 을의 담당자들이 많은 경우에 10명정도로 5~6시간 휴식없이 한적도 있습니다.
코딩보다 준비 문서량이 5배이상인 경우가 많으며, 끝나고 나면 버그 수정 내용과 수정날짜까지 정해서 회람을 할 것인지, 다시 리뷰를 할 것인지를 정합니다.
버그나 개선 사항에 따라 클레임도 있기에 상당히 긴장감 속에서 여러번 소스나 주석등을 확인하지만, 어김없이 버그가 둘이상 발견되거나 설계 문서의 오류등이 발견되더군요.(주석이나 들여쓰기가 잘못되어도 버그!)

그런 지적 사항들이 개발자들에게는 좋은 도움이 된다고 생각합니다. 보다 높은 경지에 있는 분들의 경험이나 몰랐던 내용들을 들을 수 있는 세미나라고 생각하면 한결 마음은 가벼워집니다만, 으~ 누군가 저의 소스를 보는 것은 웬지 숨기고 싶은 치부를 들키는 느낌이라.

저의 의견은, 꼭 소스 리뷰를 스케쥴에 넣는 것이 버그 발생의 줄임, 책임 분담, 수정 사항을 줄일 수 있는 확실한 방안이므로 실시를 하는 것이 좋다고 봅니다. 다만, 준비 문서를 줄이고 책임자들이 꼭 참석하길 바랍니다.