Google: Speed of Code Reviews

Google, основываясь на многолетнем опыте, представил свои стандарты того, как лучше всего проводить code review. Все вместе они представляют собой один законченный документ, разбитый на множество отдельных разделов. Это перевод стандартов по проведению Code Review от Google. Вот ссылка на оригинал!

Терминология:

CL — расшифровывается как «список изменений» (“ChangeList”), что означает одно автономное изменение, которое было отправлено в систему управления версиями или подвергается проверке кода. Другие организации часто называют это «изменением» или «патчем».

Почему Code Review следует делать быстро?

В Google мы оптимизируем скорость командной разработки, а не скорость отдельных разработчиков. Скорость отдельного разработчика важна, но не настолько, как скорость всей команды.

При медленном code review происходят следующие вещи:

• Скорость разработки команды в целом снижается. Понятно, что если ревьюер долго не отвечает на code reivew, он выполняет другую работу. В то же время, новые фичи и исправления ошибок для остальной команды откладываются на дни, недели или месяцы, так как каждый CL ожидает рассмотрения.
• Разработчики начинают протестовать против процесса проверки кода. Если ревьюер отвечает только через несколько дней и каждый раз запрашивает серьезные изменения в CL, это может быть неприятно и сложно для разработчиков. Зачастую это выражается в жалобах на то, насколько «строг» ревьюер. Однако если ревьюер запрашивает те же существенные изменения (изменения, которые действительно улучшают работоспособность кода) и всегда быстро реагирует, то жалобы, как правило, исчезают. Большинство жалоб на процесс проверки кода на самом деле разрешается путем ускорения процесса.
• Может пострадать качество кода в репозитории. При медленных обзорах кода, возникает дополнительное давление на разработчиков, что может сказаться на качестве CL. Также медленные обзоры препятствуют очистке кода, рефакторингу и дальнейшим улучшениям существующих CL.

Как быстро необходимо выполнять code review?

Если вы не находитесь в середине своей текущей задачи, то вам следует рассмотреть CL вскоре после его отправки вам. Один рабочий день — это максимальное время, которое требуется для ответа на запрос проверки кода. Следование этим рекомендациям означает, что обычный CL должен получить несколько итераций code review (при необходимости) в течение одного дня.

Скорость VS прерывания

Есть один момент, когда приоритет вашей задачи превосходит приоритет code review. Если вы находитесь в середине своей задачи, такой как написание кода, не отвлекайтесь для code review. Исследования показали, что после отвлечения разработчику требуется много времени, чтобы снова вернуться к процессу прерванной разработки. Поэтому отвлекать самого себя обойдется в целом дороже для команды, чем просьба немного подождать с code review.

Вместо этого дождитесь логического перерыва в своей работе, прежде чем отвечать на запрос о code review. Например, после завершения текущей задачи, после обеда или возвращения с собрания и т.д.

Быстрые ответы

Когда мы говорим о скорости code review, нас интересует время ответа (отклика), а не то, сколько времени занимает рассмотрение всего CL. В идеале, весь процесс также должен быть быстрым, но намного важнее быстрые отклики, нежели чем весь процесс рассмотрения CL.

Даже если весь процесс рассмотрения CL занимает много времени, быстрые ответы ревьюера значительно облегчают работу разработчикам.

Если вы слишком заняты, чтобы выполнить полный обзор CL при его поступлении, то стоит ответить разработчику о своих сроках или дать дополнительные инструкции.

Важно, чтобы ревьюеры тратили достаточно времени на рассмотрение, чтобы быть уверенными, что их «LGTM» (“looks good to me”) означает «этот код соответствует нашим стандартам». Тем не менее, отдельные ответы в идеале должны быть быстрыми.

Code review при различных часовых поясах

В таких случаях попробуйте связаться с разработчиком, когда он еще в офисе. Если он уже ушел домой, постарайтесь отправить свой ответ или комментарии прежде, чем он вернется в офис на следующий день.

LGTM (“looks good to me”) с комментариями

Для ускорения проверки кода, существуют определенные ситуации, в которых ревьюер должен дать свой аппрув, даже если есть неразрешенные комментарии к CL. Это делается в следующих случаях:

• Ревьюер уверен, что разработчик должным образом учтет все оставшиеся комментарии.
• Остальные изменения незначительны и не должны быть сделаны разработчиком.

Ревьюер должен указать, какой из этих вариантов он имеет ввиду, если это не ясно.

Аппрувы с комментариями особенно полезны, когда разработчик и ревьюер находятся в разных часовых поясах. В противном случае может возникнуть ситуация, когда разработчик будет ждать целый день, чтобы получить “чистый” аппрув.

Большие CL

Если вам прислали на ревью довольно большой объем кода, и вы не уверены, что сможете посмотреть его в ближайшее время, то следует предложить разработчику разделить CL на несколько частей. Как правило, это возможно и очень полезно для ревьюера, хотя от разработчика требуется дополнительная работа.

Если CL не может быть разбит на более мелкие CL, и у вас нет времени, чтобы быстро просмотреть его, то следует хотя бы написать некоторые комментарии об общем дизайне CL и отправить их обратно разработчику для улучшения. Одна из ваших целей в качестве ревьюера заключается в том, чтобы всегда разблокировать разработчика или позволить ему быстро предпринять какие-то дальнейшие действия.

Улучшения code review с течением времени

Если вы будете следовать этим рекомендациям и будете строго соблюдать их, то вы обнаружите, что весь процесс code review со временем будет происходит быстрее. Разработчики узнают, что требуется для работоспособного кода, и будут отправлять вам отличные CL с самого начала, требующие все меньше и меньше времени на анализ. Ревьюеры же учатся быстро реагировать и не добавляют лишних задержек в процесс code review. Но не стоит идти на компромисс со стандартами проверки кода для предполагаемого улучшения скорости — на самом деле в конечном итоге ничего не произойдет быстрее.

Непредвиденные ситуации

Также существуют чрезвычайные ситуации, когда некоторые CL должны пройти процесс code review очень быстро, и нужно будет смягчить рекомендации по качеству кода. Тем не менее, посмотрите, что такое чрезвычайная ситуация, какие ситуации на самом деле квалифицируются как чрезвычайные ситуации, а какие нет.