Code review — как это делать в стиле Google?

Рано или поздно для каждого программиста настаёт время отвлечься от собственного кода и оценить чужой. Осознав неизбежность этой работы, вам нужно будет решить, как цензурно выразить всё, что вы думаете о рецензируемом коде. Мы расскажем, как с этой задачей справляются в Google.

Стандарты

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

Дилемма обозревателя

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

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

Главное правило

Запомните, именно вам предстоит найти баланс, позволив кодерам развиваться, и в то же время не пожертвовав качеством кода.

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

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

Наставничество

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

Принципы

• Факты и данные важнее личного мнения и персональных предпочтений.
• Стиль кода должен соответствовать принятому в вашей команде. Если какой-то момент не оговорён — оставляйте его на усмотрение кодера.
• Архитектура кода должна соответствовать принципам, лежащим в основе проекта. Если есть несколько способов решить задачу — выбирайте тот, который соответствует этим принципам. Если есть несколько равно эффективных вариантов — оставьте выбор за автором кода.
• Можно попросить автора согласовать стиль кода с текущей кодовой базой, если это не ухудшит общего качества.

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

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

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

Что нужно проверить в рецензируемом коде

• Общая структура. Как код вписывается в ваш проект.
• Функциональность. Способен ли код полностью удовлетворить поставленные задачи.
• Удобство. Интуитивность UI и соответствие его общему стилю.
• Многопоточность. Рецензируемый код не должен конфликтовать с другими элементами кодовой базы при многопоточном выполнении. То же касается и внутренних конфликтов кода.
• Простота. Код не должен быть слишком громоздким. Максимально упрощаем, но не в ущерб качеству и функциональности.
• Перспектива масштабирования. Возможно, в коде могут быть реализованы некоторые возможности, востребованные в будущем, сообщите об этом разработчику.
• Наличие тестов (модульные, интеграционные и так далее). Внимательно изучите их структуру.
• Преемственность разработчиков. Все переменные, поля, функции, вообще все объекты и элементы в коде должны иметь ясные, однозначные имена. Комментарии к коду ясно и чётко объясняют, зачем нужен каждый элемент. Обратите внимание, вопрос «зачем это» важнее вопроса «что это».
• Соответствие стандартам. Код должен соответствовать стандарту стиля и быть должным образом документирован.

Проверьте каждую строчку кода, рассмотрите весь код в контексте проекта. Удостоверьтесь, что он способствует улучшению кодовой базы. Ну и не забудьте поблагодарить разработчика за интересные решения.

Последовательность действий при рецензировании кода

1. Оцените, имеют ли смысл предложенные изменения. Возможно, кодер пытается улучшить фичу, от которой ваш проект собирается избавиться. Направьте его усилия в нужное русло.
2. Изучите главную часть рецензируемого кода. Если затрудняетесь выделить эту главную часть, спросите разработчика. Найдя какие-то недостатки, немедленно сообщите автору кода. Переходить к изучению деталей кода до исправления основных недочётов нет смысла, вполне вероятно, что впоследствии эти детали значительно изменятся.
3. Изучите остальную часть кода. Ориентируйтесь на логическую последовательность действий и проверьте каждый файл, возможно, сначала имеет смысл разобрать модульные тесты, чтобы иметь представление о том, какие именно изменения планирует внести разработчик.

Скорость подготовки code review

Чем чреваты медленные code review?

• Уменьшается скорость работы всей команды. Как цепь не сильнее самого слабого звена, так и команда не быстрее самого медленного её участника.
• В Google считают, что обозреватель, появляющийся раз в несколько дней с ворохом комментариев, вызывает раздражение и жалобы разработчиков. В компании считают, что гораздо эффективнее незамедлительно реагировать на каждый апдейт от кодера, предлагая пусть и небольшие, но положительно влияющие на качество кодовой базы изменения.
• Чем сильнее вы затягиваете с code review — тем сильнее желание добавить код в базу «как есть», а это чревато ухудшением общего качества.

Как быстро нужно делать code review?

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

Единственное исключение из этого правила — если вы сконцентрированы на другой задаче. Переключение займёт слишком много времени. 

Постепенно вы сможете увеличить скорость работы. Но не делайте это в ущерб качеству.

Работа с большими объёмами

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

Экстренные ситуации

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

Как писать комментарии во время рецензирования

• Будьте благожелательны. Вы с разработчиком на одной стороне баррикад. Рецензируйте код, а не кодера.
• Не будьте императивны, поясняйте свои рекомендации и указания. Это поможет разработчику лучше понять и выполнить их.
• Объясните, чего вы хотите, но не делайте работу за кодера. Как правило, он всё же может сделать её лучше. Помните, разработчику нужно дать возможность развиваться. Опять же, не в ущерб качеству. Ищите баланс.
• Предложите разработчику попробовать упростить код, а если это невозможно — добавить подробные комментарии.

Работа с возражениями

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

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

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