Google: The Standard of Code Review

Albert Davletov
UniLecs
Published in
3 min readSep 12, 2019

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

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

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

Стандарт проверки кода

Основная цель проверки кода — убедиться, что общее состояние кода со временем улучшается. Все инструменты и процессы проверки кода предназначены для этой цели.

  • Во-первых, разработчики должны быть в состоянии добиться прогресса в выполнении своих задач. Если вы никогда не отправляете улучшение в ваш репозиторий, то репозиторий никогда не улучшается (спасибо кэп). Кроме того, если ревьюер затрудняет внесение каких-либо изменений, разработчики не заинтересованы в том, чтобы вносить улучшения в будущем.
  • С другой стороны, ревьюер обязан следить за тем, чтобы каждый CL был такого качества, чтобы общее состояние кода его репозитория не ухудшалось с течением времени. Это может быть непросто, потому что часто репозитории ухудшаются из-за небольшого снижения работоспособности кода с течением времени. Это случается, когда команда испытывает значительные ограничения по времени.

Кроме того, ревьюер имеет право собственности и ответственность за код, который он проверяет. Он стремится к тому, чтобы репозиторий оставался последовательным и поддерживаемым. Таким образом, мы получаем следующее правило как стандарт, который мы ожидаем в обзорах кода:

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

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

Ключевым моментом здесь является то, что не существует такого понятия, как «идеальный» код — есть только более качественный код. Ревьюеры не должны требовать, чтобы автор полировал каждый крошечный кусочек CL перед “апрувом”. Скорее, ревьюер должен сбалансировать необходимость продвижения вперед с важностью изменений, который предлагает автор. Ревьюер должен стремиться не к совершенству, а к постоянному совершенствованию. CL, который в целом улучшает удобство обслуживания и понятность системы, не следует откладывать на несколько дней или недель, потому что он не «идеален».

Ревьюеры всегда могут свободно оставлять комментарии с рекомендациями по улучшению кода. Однако если рекомендации не столь важны, добавлют к нему что-то вроде «Nit:», чтобы дать понять, что это всего лишь точка зрения, которую авторы могут игнорировать.

Менторство

Проверка кода может быть важным аспектом в обучении разработчиков чему-то новому о языке, структуре или общих принципах проектирования программного обеспечения. Всегда полезно оставлять комментарии, которые помогают разработчику узнать что-то новое. Обмен знаниями является частью улучшения работоспособности кода системы с течением времени. Просто имейте в виду, что если ваш комментарий носит чисто образовательный характер, но не критичен для соответствия стандартам, добавьте к нему префикс «Nit:» или иным образом укажите, что автору не обязательно разрешать его в этом CL.

Принципы

  • Технические факты и данные превалируют над мнением и личными предпочтениями.
  • Руководство по стилю является абсолютным авторитетом. Любая чисто стилевая точка (пробел и т.д.), которой нет в руководстве по стилю, является вопросом личных предпочтений. Стиль должен соответствовать тому, что есть. Если предыдущего стиля нет, то примите авторский.
  • Аспекты разработки программного обеспечения почти никогда не являются чисто стилевым вопросом или личным предпочтением. Они основаны на основополагающих принципах и должны оцениваться на этих принципах, а не просто по личному мнению. Иногда есть несколько действительных вариантов. Если автор может продемонстрировать (либо с помощью данных, либо на основе основательных инженерных принципов), что несколько подходов одинаково действительны, то ревьюер должен принять предпочтение автора. В противном случае выбор продиктован стандартными принципами проектирования программного обеспечения.
  • Если никакое другое правило не применяется, то ревьюер может попросить автора использовать тот же стиль, который используется в текущем репозитории, если это не ухудшает общее состояние кода системы.

Разрешение конфликтов

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

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

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

--

--