코드 리뷰는 어떻게 하나요?

후배가 구글에서는 코드 리뷰를 어떻게 하면 좋을지를 물어보는 이메일을 보내 왔습니다. 저도 평소에 꼭 공유하고 싶었던 주제여서 여기에 답글을 남깁니다.

질문은 구글에서 어떻게 하느냐인데, 답글은 구글에 국한된 부분은 아니고 실리콘 밸리 회사들이 주로 어떻게 하는지 설명 드리겠습니다.

--------

선배님께 이렇게 메일을 보내는 이유는 한 가지 여쭤볼게 있어서 입니다.
요즘 회사일을 하든, 친구들과 서비슬 개발을 준비하든, 학교와는 달리 다른사람과 협업해서 코드를 짜야하는 경험을 하고 있습니다.
그래서 동료들 간에 코딩 컨벤션을 정하거나, 코드 리뷰를 할 필요성을 느끼게 되었고, 최고의 팀웍을 위해서 잘하고 싶은데 말처럼 쉽지가 않습니다. 예를 들면 리뷰어가 리뷰를 할 때 리뷰를 받는 사람은 왜 그게 더 좋은지 인지를 못하거나 혹은 쌍방 감정싸움으로 이어지지 않게 하려다 보니 대충대충 넘어간다거나 하더라구요...ㅠㅠ


그래서 질문은 다음과 같습니다.


1. 구글에서는 내부 구성원 사이에 코딩컨벤션을 어떻게 전달하고 공유하나요?
(위키 정리, 내부 스터디, 토론형 학습...등)

구글 코딩 컨벤션은 Java를 제외한 왠만한 언어는 아래 사이트에 공개 되어 었습니다.

https://code.google.com/p/google-styleguide/

그냥 그대로 쓰거나, 필요에 맞게 조금 고쳐 쓰면 될 것 같습니다.

(한가지 특이한 부분은 대부분의 회사들은 4 space indentation을 쓰는데 구글을 2 space indentation을 씁니다. 장단점이 있구요. 개인적으로는 python같이 ending brace가 없는 언어에서 depth가 깊고 block이 길 경우 좀 불편한 것 같습니다.)

2. 코드 리뷰 전체 프로세스가 어떤식으로 진행되나요?
(리뷰신청 -> 주로 언제까지 리뷰 완료 등등, 따로 리뷰 시간을 가지는 지 등)

CL (Change List)이 준비되면 "p4 mail"이나 gerrit 같은 코드 리뷰 시스템으로 리뷰를 신청합니다. 그러면 reviewer에게 이메일로 notification이 가게 되고, reviewer가 리뷰 시스템에 들어 가서 review comment를 작성하게 됩니다.  1차 review를 완료하면 이 사실에 CL 작성자에게 이메일로 notification이 가게 되죠. 이런 식으로 왔다 같다 하면서 CL이 submit 할 수준이 되면 reviewer가 approve를 하고, 그러면 작성자가 submit을 합니다.

가끔 의견차이가 잘 좁혀 지지 않을 때나, 상대방의 말이 잘 이해가 안 될 때 만나서 얘기 하는 경우도 종종 있습니다. 글로 잘 대화가 안 되는 경우, 말로 하는 것이 훨씬 효율적이고 빠릅니다. 작성자이던 reviewer이던, 조금 이라도 막힌다는 느낌이 있으면 찾아 가서 직접 얘기하세요. 그게 훨씬 효율적입니다. 단 이 경우에도 반드시 리뷰 시스템에 "만나서 이렇게 합의를 봤고, 결론에 도달한 이유"를 간단히 적어 놓아야 합니다. 그래야 차후에 그 CL을 참조하는 사람이 왜 그렇게 했는지를 알 수 있으니까요.

구체적인 안건에 대한 의견 조율을 위해 offline으로 얘기하는 경우는 많지만, 따로 같이 리뷰 시간을 가지지는 않는 것 같습니다.

리뷰어는 가급적이면 CL을 받으면 최대한 자기일 보다는 리뷰를 먼저 해 주는 것이 좋습니다. CL 작성자의 입장에서는 리뷰를 얼마나 빨리 받는지에 따라 업무 효율이 많이 좌우 되니까요. 이건 CL을 처음으로 받았을 때도 그렇고 업데이트된 CL (보내준 review에 따라 수정한)을 받았을 때도 마찬가지 입니다.

보통 일반적인 규칙은 (처음이든 update한 버젼이든 상관 없이) CL을 받으면 24시간안에는 리뷰를 보내준다 입니다. 하지만 이건 hard deadline에 가깝고, 대개는 그날 받은 CL은 그날 리뷰해 주는 것이 좋습니다. (오후 늦게 받은 건 그 다음날 오전에 리뷰 해 줘도 괜찮습니다.)

reviewer가 어뗜 이유로 - 아주 급한 일이 있다던지, CL이 너무 크고 이해하기 어렵다든지 해서 - 24시간안에 리뷰를 못 보내줄 것 같으면, 이메일이나 offline으로 그 사정을 CL작성자에게 얘기를 미리 해 주는 것이 좋습니다. 예를 들어 "이런 저런 사정으로 x일까지 리뷰 해 줄 수 있는데 괜찮겠냐?"고.

반대로 CL작성자가 리뷰를 받고 언제까지 수정 버젼을 보내줄 것인지에 대한 규칙은 없습니다. 이게 늦어지면 대체로 CL 작성자가 손해를 보는 쪽이니까요. 그래도 아주 극단적으로 늦어지는 경우 (1~2주?) reviewer가 문맥(context)를 잊어 버리니까, "늦게 보내서 미안하다" 정도는 적어 주는 것이 좋습니다.

3. 코드 리뷰는 어느 정도까지 하나요?
(테스트 코드 여부, 로직 검토, 변수 네이밍, 공백 등등)

이 질문에 대한 답은 상황에 따라 많이 다릅니다. 예를 들어 이미 대규모로 서비스 되고 있거나 곧 서비스 될 경우, 또는 큰 과제여서 많은 사람들이 공동 작업하는 코드, 중요한 과제여서 코드가 향후 오랫동안 사용될 경우 등에는 code quality가 아주 중요합니다. 따라서 리뷰도 더 깐깐하게 해야 합니다. 또한 주요 로직에 대해서는 unit test가 다 있어야겠죠. 때문에 "이런 저런 테스트를 추가해 주세요"는 상당히 당연한 리뷰 comment 입니다.

반면, 실험적인 코드, 1~2명이 작업하는 코드, 향후 버려질 가능성이 높은 코드는 code quality 보다는 속도가 더 중요할 수 있습니다. 이 경우 리뷰를 너무 깐깐하게 하면 얻는 것보다 비용이 더 많이 들 수도 있습니다. 때문에 약간 리뷰를 느슨하게 하는 것도 괜찮습니다. 극단적인 경우 - 완전히 실험적인 코드 - 는 아예 리뷰를 안하는 경우도 있습니다. 단 이 경우는 repository 나 디렉토리를 완전히 분리하고, 거기에는 리뷰 안 한 코드가 들어있다는 것을 사전에 팀 전체가 공유 해야 합니다. (이런 코드는 리뷰를 하는 코드가 들어 있는 디렉토리에서 불러 써도 안되죠.)

위의 두 가지 경우의 리뷰 모두,  스타일 가이드는 기본적으로 따라야 합니다.  일단 lint 같은 툴로 대부분의 문제는 다 걸러 내고, lint로 안 잡히는 문제는 reviewer가 comment를 줍니다. 단 이 경우 너무 style guide를 바이블처럼 따를 필요도 없고, 그래서도 안됩니다. style guide를 100% 지킬려고 하다 보면 너무 많은 에너지가 들 수 있거든요. 하지만 reviewer는 style guide에 안 맞는 것이 보이면 comment를 달아 주는 것이 맞고, 작성자도 그런 comment는 다 따라 주는 것이 좋습니다. 왜냐 하면 보통 style guide 관련 comment들은 고치는데 별로 시간이 많이 안 걸리니까요.

불필요한 공백(extra whitespace)은 다 없애는게 맞습니다. 안 그러면 나중에 리뷰 시스템에서 diff로 볼 때 바뀌지 않은 라인이 바뀐걸로 나오니까요. editor에서 "저장할 때 extra whitespace 없애는" 옵션을 항상 켜두면 편합니다.

comment를 달 때, 반복적인 문제들에 대해 다 comment를 달 필요는 없고, "이 라인에 이런 문제가 있고 다른 곳에도 반복되니까 모두 고치세요" 라고 하면 됩니다.

변수, 함수 이름은 코드를 이해하는데 중요하죠. 따라서 "지금 이름보다 이 이름이 더 명확할 것 같은데 어떠세요?"라고 충분히 얘기할 수 있습니다. 하지만 현재의 이름도 꽤 괜찮다면 너무 완벽하게 하기 위해서 고쳐 주세요 할 필요는 없습니다. 반면 외부에 공개 되는 API라던지, 외부에 공개 되지는 않지만 내부적으로 아주 많이 쓰일 것 같은 API는 조금이라도 더 좋아질 여지가 있으면 고치는 게 낫습니다.

review를 할 때, 중요한 것 중 하나는 CL의 전체적인 디자인이 괜찮은지를 보는 것입니다. 더 단순한 디자인은 없는지, 이해하기 쉬한 구조인지, 확장성, 유연성은 좋은지, 테스트는 쉬운지 등을 잘 보셔야 합니다. 특히 테스트가 쉬운 구조인지 (Testability)는 특히 중요한 항목입니다. 설령 당장 unit test가 없더라도 중요합니다. 나중에 테스트를 추가할려고 할 때 쉽게 할 수 있어야 하니까요. 그리고 일반적으로 Testability 가 안 좋은 코드는 다른 문제도 많이 가지고 있으니까요.

4. 코드 리뷰를 할 때 어떤 식으로 코멘트를 달아주나요?
("이건 이 방식으로 하시오", "이 라이브러리를 쓰시오", "이 이름이 더 좋아" 등등)

말씀 하신 것과 내용은 동일한데 훨씬 부드럽게 쓰는 경우가 많은 것 같습니다. 예를 들어, 영어로 "Could you ...", "How about ...", "Please ... "이런 표현도 많이 씁니다.

스타일 가이드에 안 맞거나, 더 좋은 라이브러리, 디자인을 제안하는 경우에는 관련 설명이 되어 있는 문서의 링크를 넣어서 왜 그렇게 생각하는 지를 알려주는 것도 좋습니다. (작성자도 알 것 같은 것은 굳이 이렇게 안 해도 됩니다.)

5. 코드 리뷰에 대한 사람들의 인식, 태도, 문화가 어떻나요?
(서로 배운다는 인식을 가진다, 선배 뜻을 따른다... 등)

실제로 코드 리뷰를 통해서 가장 많이 배웁니다. 코드 리뷰를 통해서 나보다 더 나은 엔지니어로 부터 구체적인 코드에 대해서 더 나은 디자인이 어떤 것이고, 또 더 나은 코드는 어떤 것인지 바로 바로 배울 수 있으니까요.

그리고 코드 리뷰를 통해서 code quality가 유지 되어야 내가 쉽게 그 코드 기반에서 작업할 수 있으니까요.

6. 기타 코드 리뷰와 관련해서 노하우 같은 게 있으신가요?

• 주위에 나 보다 뛰어난 엔지니어가 있으면, 그 분들한테 리뷰를 많이 받으세요. 그러면 실력이 일취월장하게 됩니다.

• 나와 비슷하거나 나보다 좀 못하는 엔지니어에게 코드 리뷰를 받더라도 보는 관점이 다르기 때문에 항상 배우는 점이 있습니다. 그런 열린 마음이 중요합니다.

• 리뷰시에는 나만의 스타일을 고집하면 안 됩니다. 스타일 가이드에 있는 것은 잘 따라야지만, 그 이외의 부분에 대해서는 사람마다 다 스타일이 다르기 때문에 스타일에 관한 comment는 하시면 안 됩니다. 내가 할려는 comment가 code quality를 향상 시키려는 것인지, 스타일 문제인지를 잘 생각해 보세요.

• CL 작성자가 고집이 세서, 분명히 타당한 comment인데도 받아들이지 않는다면, 한두번 얘기해 보고 안 되면 포기 하세요. code quality는 떨어지겠지만, 그게 팀원들끼리 감정 상하는 것보다는 훨씬 낮습니다. 그리고 이런 일이 특정 작성자와 반복되면 직접 만나서 허쉼탄회하게 얘기를 해 보는 것도 괜찮습니다. 단 감정이 상하지 않게 잘 해야 합니다. 그리고 너무 기대를 마세요. 사람은 안 바뀌니까요.

• 같은 말도 "아" 다르고 "어" 다르듯이, 상호 코멘트를 달 때 예의를 갖추고 해야 합니다. 그리고 상대가 잘한 부분에 대해서는 칭찬하는  comment를 많이 달아 주세요. 예를 들어 "이 CL 정말 대단한데요.", "이 CL 정말 있었으면 좋겠다고 항상 생각해 왔는데 이거 만들어 주셔서 정말 감사합니다.", "이 디자인은 ....해서 정말 훌륭하군요.", "이 테스트는 정말 coverage가 대단 하네요.", "제안해 주신 디자인은 .. 해서 훨씬 낫군요" 등등.

• 역지사지 - 작성자편: reviewer가 쉽고 빨리 리뷰 할 수 있는 CL을 만들도록 노력하세요. 내 시간이 중요한 만큼 reviewer의 시간도 중요합니다. 한가지 방법은 CL을 가능한 작게 만들는 것입니다. CL 하나에는 하나의 기능만 담으세요. 그래야 리뷰가 쉽고 빨리 리뷰를 할 수 있습니다. 이렇게 하면 나중에 코드 history를 살펴볼때도 편하고 여러 가지로 좋습니다. 또 한 가지 방법은 CL 보내기 전에 자신이 리뷰어가 됐다고 생각하고 리뷰 시스템에서 한 번 훑터보고 보내는 것입니다. 자신의 에디터에서 볼 때 못 보았던 문제를 많이 발견하게 될 겁니다. 반면 가장 안 좋은 자세는 대강 CL을 만들어서 리뷰 보내고, reviewer가 comment를 보내면 그걸 고치면서 CL을 완성하겠다는 자세입니다. 내 시간이 중요한 만큼 reviewer의 시간도 중요합니다. 최대한 완성된 CL을 보내세요.

• 역지사지 - reviewer편: 리뷰시 해서 안되는 것 중에 하나는 CL의 범위가 아닌것을 이것 저것 고치라고 하는 겁니다. 예를 들어 현재 리뷰하는 CL 이전부터 기존에 갖고 있는 코드의 문제를 이번 CL에서 고치라고 하는 겁니다. 또 다른 해서는 안되는 것은 너무 무리한 것을 강요하는 것입니다. 예를 들어, "현재 CL에서 빠진 unit test가 있는데, 이걸 추가를 하면 좋지만 추가 하는데 너무 노력이 많이 든다"면, 일단 현재 CL에서는 TODO로 남기고 그 다음 CL에서 하는 것도 방법입니다. 작성자가 이렇게 하자고 하는데도, "unit test 추가 안 하면 approve 안 해 주겠다" 이런 것은 좋은 태도가 아닙니다. 반면, 비용이 많이 들지만 code quality가 중요한 과제이고, 제안하는 테스트가 중요한 것이라면, 이번 CL에서 하는 것이 맞는 경우도 분명 있습니다. 

• 인간 관계에서도 첫인상이 중요하듯이 코드 리뷰 역시 첫인상이 중요합니다. 자신의 코드를 처음 리뷰하는 reviewer에게 CL을 보낼 경우 조금 더 신경을 쓰세요. 처음 보내는 CL quality가 낮아서 여러 가지 지적을 받게 되면, 그 reviewer가 작성자에 대해 잘못된 선입관을 가질 수 있고, 이후의 CL들에 대해서도 불필요하게 더 깐깐하게 리뷰할 수도 있습니다.

• 아직 제품이 시장에서 검증이 안 된 스타트업의 경우, 전략적으로 상당한 기술적 빚 (technical debt)를 안고 가는 것도 괜찮은 방법입니다. 이 경우 code quality는 어느 정도 포기하고, 코드 리뷰에 들어가는 노력을 작게 가져 갑니다. (린스타트업이란 책을 참고 바랍니다.)

길게 글을 썼는데 적다 보니, 코드 리뷰를 배우는 가장 좋은 방법은, 리뷰를 잘 하는 사람에게 리뷰를 받는 것이라는 생각이 계속 드네요. 주변에 리뷰를 잘 하는 분이 있으면 그 분에게 리뷰를 받으면서 디자인, 코딩 실력도 키우시고, 동시게 리뷰 실력도 키우시기 바랍니다.

학생이라면 방학 때 실리콘 밸리 회사에서 인턴을 하는 것도, 코드 리뷰를 배우는 좋은 방법입니다 (그 밖에도 많은 것들을 배울 수 있지요). 구글 같은 큰 회사는 비행기표, 숙박 제공하고 월급도 상당히 높답니다. 우리 나라 회사라도 코드 리뷰를 제대로 하는 회사나 팀에서 인턴을 하는 것도 괜찮습니다. 코드 리뷰를 제대로 한다면 전반적으로 꽤 괜찮은 기술력을 가지고 있을 거라서 다른 것도 많이 배울 수 있구요.