Руководство по код-ревью для начинающих

Перевод статьи «The Code Review Guide».

Код-ревью это обыденная практика в индустрии технологий. Когда вы делаете пул-реквест (мерж-реквест), кто-то должен его проверить, дать фидбэк и одобрить, когда код будет готов стать частью основной ветки (если, конечно, пул-реквест касался именно этой ветки).

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

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

С этой установкой мы и приступим к обзору.

Вам нужен контекст

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

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

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

Окей, мы уже знаем, какова тема пул-реквеста. Давайте посмотрим код. Его можно проверять по двум направлениям: по читаемости и по функционалу. Что бы вы ни выбрали, постоянно держите в уме два вопроса:

• Как я могу это улучшить?
• Что может пойти не так?

Проверка на читаемость

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

Каждый кусок кода, который вы пишете, имеет свою цену — поддержку. Сделайте окружающим одолжение и проверяйте свой код тщательно.

Вот несколько вещей, которые можно проверить:

• Поищите опечатки (они встречаются повсеместно) в именах переменных и функций, в комментариях, описаниях и даже в именах файлов.
• Имена классов и функций должны быть хорошо подобраны. Имя функции должно отражать то, за что она отвечает.
• Комментарии должны быть понятными и хорошо написанными. Но это не значит, что они должны быть длинными.
• Постоянство. Код не пишется как отдельный, изолированный отрывок: он является частью кодовой базы. Убедитесь, что при написании кода автор придерживался общепринятых подходов. Постарайтесь проверить, соответствует ли этот код всему остальному коду в базе. Например, в нем могут отличаться стили написания составных слов:

// camelCase
// PascalCase
// snake_case
// kebab-case

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

• В коде не должно быть print-ов. Если вы их находите, вероятно, автор забыл удалить их после дебаггинга.
• Избегайте ненужной вложенности.
• При возможности результат должен возвращаться как можно раньше. Если видите что-то такое:

var myVar = "I am a variable"

if strings.ContainsAny("a", myVar){
   return myVar
}else{
   return nil
}

вы можете это изменить следующим образом:

var myVar = "I am a variable"

if strings.ContainsAny("a", myVar){
   return myVar
}

return nil

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

Проверка функциональности

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

• Есть ли смысл в этом потоке?
• Есть ли здесь ненужные проверки, циклы и т. п.?
• Помните о принципе единственной ответственности для методов и классов. Благодаря этому тестирование будет более атомарным.
• Возможно ли, что эта функция «упадет»? Например, проверьте, что длина массива проверяется перед осуществлением доступа к нему. Или по крайней мере убедитесь, что дело не кончится ошибкой -NullPointer alert-.
• Как бы я это сделал? Лучше ли мой путь? Может ли эта логика улучшить существующую?
• Тесты не меняются без причины. Например, если тест использовался для проверки того, что функция не выбрасывает ошибку, но затем тот же тест проверят, что функция выбрасывает ошибку, — спросите, почему. Возможно, произошли какие-то важные изменения, повлиявшие на эту операцию.
• Также тесты не удаляются без причины. Если не знаете, почему тест был удален, — спросите. Что, он больше не нужен? А почему?
• Чтобы проверить вносимые изменения и их влияние, тестов достаточно. При этом убедитесь, что тесты подтверждают то, что должны подтверждать.

Что делать, если я не знаю язык, на котором написан код?

Это классика. Вы не знаете язык или технологию, но должны проверить код, где они используются.

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

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

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

И самый главный совет: не нужно слепо следовать «Best Practices» («лучшим подходам»). Всегда сначала спрашивайте. Каждый кусок кода должен увеличивать ценность продукта. Думайте о том, каким образом этот код улучшает продукт, а также старайтесь облегчить жизнь разработчику, которому в будущем придется этот код поддерживать (возможно, это будете вы сами).

[customscript]techrocks_custom_after_post_html[/customscript]

[customscript]techrocks_custom_script[/customscript]