코드 리뷰

예전에는 각자 만든 코드가 아무도 거치지 않고 본류에 바로 들어갔습니다. 문제는 두 가지 방향에서 터졌습니다. 하나는 실수가 만든 사람 눈에 가장 안 보인다는 점입니다. 같은 화면을 세 시간 들여다본 사람은 자기가 무엇을 빠뜨렸는지 알아채지 못합니다. 다른 하나는 그 코드를 이해하는 사람이 세상에 한 명뿐이라는 점입니다.

페이지덱에서도 같은 일이 있었습니다. 결제 부분을 태오가 혼자 만들어 넣었고 석 달 동안 아무 일 없이 돌아갔습니다. 그러다 특정 카드만 결제가 밀리는 증상이 생겼는데, 그 코드를 열어 본 사람이 태오밖에 없어서 태오가 휴가 중인 나흘 동안 아무도 손을 대지 못했습니다. 버그 자체보다 손댈 사람이 없다는 쪽이 더 큰 문제였습니다.

코드 리뷰는 이 둘을 한 절차로 막으려고 자리 잡았습니다. 합치기 전에 사람을 한 번 거치게 만드는 것인데, 이때 검사만 얻는 게 아니라 그 코드를 읽은 사람이 하나 더 생깁니다. 실제로 팀에 오래 남는 효과는 뒤엣것입니다.

변경을 올리는 순간부터 본류에 합쳐질 때까지 여섯 단계를 지납니다.

1. 작성자가 자기 브랜치에서 작업을 끝내고 리뷰를 요청합니다. 브랜치는 본류를 건드리지 않고 따로 작업하려고 갈라 둔 줄기입니다. 이때 무엇을 왜 바꿨는지 설명을 함께 적습니다.
2. 리뷰어를 지정합니다. 보통 한두 명이고, 팀 전원을 부르지 않습니다. 사람이 많아질수록 서로 미루기 때문에 세 명 이상 붙이면 오히려 응답이 늦어집니다.
3. 자동 검사가 먼저 돕니다. 린터가 들여쓰기나 변수 이름 같은 규칙 위반을 잡고, 단위 테스트가 함수 하나하나의 동작이 그대로인지 확인합니다. 기계가 잡을 수 있는 것을 사람이 지적하는 데 시간을 쓰지 않으려는 배치입니다.
4. 리뷰어가 바뀐 줄을 읽고 의견을 답니다. 줄 옆에 직접 붙이므로 "저 파일 어딘가" 가 아니라 정확히 그 자리를 가리킵니다.
5. 작성자가 고치거나 왜 그렇게 두었는지 답합니다. 모든 의견을 그대로 반영해야 하는 것은 아닙니다.
6. 리뷰어가 승인하면 합칩니다. 승인이 없으면 본류에 들어가지 못하도록 막아 두는 것이 기본 설정입니다.

읽는 쪽이 보는 것은 셋입니다. 의도한 대로 도는지, 흔치 않은 입력이 들어왔을 때 무엇이 터지는지, 여섯 달 뒤에 처음 보는 사람이 읽어 낼 수 있는지입니다. 앞의 둘은 지금 동작이고 뒤엣것은 이 코드를 얼마나 오래 고쳐 쓸 수 있는지입니다.

의견은 세기를 구분해서 답니다. 안 고치면 안 되는 것, 고치면 좋겠다는 제안, 단순한 질문 셋을 말머리로 표시해 두면 작성자가 무엇부터 봐야 할지 압니다. "여기 null 이면 터집니다" 와 "이 변수 이름은 저라면 다르게 썼을 것 같아요" 가 같은 무게로 보이면 작성자는 둘 다 지시로 받아들이고 사소한 것부터 고치기 시작합니다.

한 번에 올리는 양은 400줄 안쪽이 기준입니다. 200줄짜리 변경에서는 리뷰어가 줄마다 읽지만, 1,200줄이 올라오면 대부분 앞의 100줄만 꼼꼼히 보고 나머지는 훑고 승인합니다. 크게 올릴수록 통과는 빨라지고 걸러지는 문제는 줄어듭니다.

리뷰어는 그 영역을 아는 사람 한 명과, 앞으로 그 코드를 만질 가능성이 있는 사람 한 명으로 짭니다. 앞엣사람이 검사를 맡고 뒤엣사람이 전파를 맡는 셈입니다. 연차 순서로 정하지 않습니다. 들어온 지 두 달 된 사람이 5년 차 코드를 읽고 "이 부분이 왜 필요한지 모르겠습니다" 라고 쓰는 것도 유효한 리뷰입니다. 설명 없이는 이해되지 않는 코드라는 사실 자체가 발견이기 때문입니다.

시점은 올라온 뒤 하루 안입니다. 페이지덱은 오전에 올라온 것을 그날 퇴근 전까지 본다는 약속을 두고 있습니다. 리뷰가 늦으면 작성자는 그동안 다음 작업을 시작하고, 나중에 의견을 받으면 이미 다른 것을 만들던 머리로 돌아와야 합니다. 브랜치도 같이 묵습니다. 하루 만에 합치면 머지할 때 겹치는 줄이 두세 개지만 일주일을 두면 그사이 본류가 움직여 충돌이 파일 단위로 커집니다.

예외는 장애 대응입니다. 서비스가 멈춘 상황에서는 먼저 고쳐 올리고 리뷰는 그다음 날 받습니다. 다만 이 예외를 자주 쓰면 절차가 이름만 남습니다.

"이거 리뷰 좀 봐 주실 수 있을까요? 급한 건 아니고 내일까지만 주시면 됩니다."

기한을 함께 말해 상대가 지금 하던 일을 멈춰야 하는지 알려 주는 요청입니다.

"리뷰가 너무 커서 반으로 쪼개 주시면 좋겠어요."

지금 크기로는 제대로 읽을 수 없으니 제대로 읽을 수 있는 크기로 다시 달라는 뜻입니다.

"이건 제안이라 안 고치셔도 됩니다. 승인 눌러 둘게요."

의견은 남기되 합치는 것을 막지는 않겠다는 표시입니다.

페이지덱에서는 리뷰가 개발팀 하루의 시작입니다. 태오는 출근해서 자기 작업을 잡기 전에 올라온 요청부터 읽습니다. 자기 것이 리뷰를 기다리는 동안 남의 것을 보는 순서가 자연스럽게 잡히기 때문입니다.

지현이 이 절차를 보는 이유는 조금 다릅니다. 일정이 밀릴 때 원인이 개발 속도가 아니라 리뷰 대기인 경우가 있어서입니다. 작업은 화요일에 끝났는데 승인이 금요일에 났다면 사흘은 아무도 일하지 않은 채 흘러간 시간입니다. 이 구간이 반복해서 길면 지현은 사람을 더 붙이자고 하지 않고 리뷰 순서를 먼저 손봅니다.

리뷰를 잘못 찾아내는 자리로 읽는 경우가 가장 많습니다. 받는 쪽에서는 자기 결과물에 빨간 줄이 그어지는 것처럼 보이니 그렇게 느끼기 쉽습니다. 실제로는 코드를 함께 소유하는 절차에 가깝습니다. 승인한 사람도 그 코드에 책임을 나눠 지고, 그래서 나중에 그 부분이 문제가 되면 작성자만 불려 가지 않습니다. 이 오해가 자리 잡은 팀에서는 사람들이 리뷰받을 일을 피하려고 변경을 몰래 크게 키우거나, 의견을 남기지 않고 승인만 누릅니다. 그러면 절차는 남고 효과만 사라집니다.

리뷰를 통과했으니 버그가 없다고 읽는 것도 흔합니다. 승인 표시가 품질 보증처럼 보이니 자연스러운 오해입니다. 리뷰어는 코드를 읽을 뿐 실행해 보지는 않기 때문에 잡히는 것은 읽어서 보이는 문제뿐입니다. 특정 조건에서만 나타나는 오류나 데이터가 많아질 때 느려지는 문제는 읽어서는 보이지 않습니다. 리뷰를 최종 관문으로 믿는 팀은 테스트와 검수 단계를 얇게 두고, 그 상태로 반년쯤 지나면 운영 환경에서만 발견되는 장애 비율이 눈에 띄게 올라갑니다.