Navigating a CL in review

본 문서는 google code review guide를 보고 요약한 글 입니다.

여러 파일에 걸친 리뷰를 관리하는 가장 효율적인 방법은 무엇일까요?

1. 변경한 부분이 의미가 있는지, 그것에 대한 좋은 설명이 있는지 확인합니다.
2. 변경한 부분의 가장 중요한 부분을 먼저 읽습니다. 그 부분은 전체적으로 설계가 잘 되어있는지 확인합니다.
3. CL의 나머지 부분을 적절한 순서로 읽습니다.

1. Step One: 변화를 폭 넓게 고려하기 (Take a broad view of the change)

우선 CL의 description을 읽고 CL에서 변경된 동작(기능)을 확인합니다. 변경한 부분이 의미가 있나요? 만약 이런 변경이 일어나지 않았어야 하는경우, 그것에 대해 설명을 통해 즉시 응답해야합니다. 이와 같은 변경한 부분에 대해 리뷰어가 거부 의사를 표시한 경우 코드 작성자에게 대신 수행해야할 작업을 제안 하는 것이 좋습니다.

예를들어 다음과 같이 말할 수 있습니다:
“감사합니다! 그러나 실제로는 당신이 수정하려는 FooWidget을 시스템에서 제거하려고합니다. 그렇기 때문에 FooWidget에 대해 새로운 변경을 하기에는 적절하지 않다고 생각합니다. 대신 새로운 BarWidget을 refactoring하는 것은 어떤가요?”

리뷰어가 현재의 CL을 거부하고 대안을 제시했을 뿐만 아니라, 이를 정중하게 했다는 점을 주의 깊에 봐야합니다. 이런 예의는 우리가 개발자로서 서로를 존중한다는 것을 보여주기때문에 매우 중요합니다.

만약 원하지 않는 변경사항이 포함된 CL이 몇 개 이상 있는 경우, 개발 프로세스를 다시 설정하여 CL이 작성되기 전에 많은 의사소통이 이루어 지도록 해야합니다. 시스템에서 삭제하거나, 과감하게 다시 써야 하는 엄청난 일을 다른 사람이 하기 전에 “no”라고 말하는 것이 좋습니다.

2. Step Two: CL의 주요 부분을 검사하기 (Examine the main parts of the CL)

CL의 주요 변경사항이 포함된 파일을 찾습니다. 논리적으로 가장 많은 수의 변경 사항이있는 파일이 CL의 주요 부분일 확률이 높습니다. 이 부분을 먼저 읽습니다. 이 부분은 CL의 모든 작은 부분에 대한 문맥(context)를 제공하고, 코드 리뷰 속도를 가속화할 수 있게 해줍니다. CL이 너무 커서 주요 부분을 알아낼 수 없는 경우, 코드 작성자에게 주요 부분에 대해 문의하거나 CL을 분할하도록 요청합니다.

CL의 주요 부분에서 중대한 설계상의 문제가 있는경우, 나머지 CL을 검토할 시간이 없더라도 즉시 의견(comment)을 남겨야합니다. 실제로 CL의 나머지 부분을 검토하는 것은 시간낭비일 수 있습니다. 설계 문제가 심각하다면, 리뷰중인 많은 코드가 사라지기 때문입니다.

주요 설계에 문제가 있을경우, 의견을 즉시 전달하는 것이 왜 중요한지에 대한 이유는 다음과 같습니다.

1. 코드 작성자는 종종 해당 CL의 리뷰를 기다리는 동안 해당 CL을 기반으로 즉시 새 작업을 시작하는 경우가 있습니다. 이는 리뷰중인 CL에 주요 설계 문제가 있는 경우, 나중에 다시 작업해야할 우려가 있습니다. 문제가 있는 설계를 토대로 너무 많은 추가 작업을 수행하기 전에 그 문제를 해결해야합니다.
2. 주요 설계 변경은 작은 변경보다 시간이 오래 걸립니다. 개발자들은 거의 모두 일정이 있습니다. 이 일정을 지키고 코드베이스의 품질을 유지하려면, 코드 작성자는 가능한 빨리 CL을 재작업 해야합니다.

3. Step Three: 나머지 부분을 적절한 순서로 살펴보기 (Look through the rest of the CL in an appropriate sequence)

CL 전체에 중대한 설계문제가 없다는 것이 파악되었다면, 파일을 리뷰할 수 있는 논리적 순서를 파악하면서 나머지를 리뷰합니다. 일반적으로 주요 파일을 살펴본 후에는 코드 리뷰 툴이 제시하는 순서대로 각 파일을 살펴보는 것이 가장 간단합니다.

때로는 주요 부분을 읽기전에 먼저 테스트 코드를 읽는것이 도움이 됩니다. 변경 사항이 무엇인지 쉽게 파악할 수 있기 때문입니다.