Как делать Code Review

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

Представьте, что вам нужно просмотреть чей-то pull request. Человек работал, что-то делал. Вы видите изменения в файлах. На что в первую очередь обратить внимание?

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

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

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

Затем осмотрите код на наличие грубых ошибок: возможно, человек где-то не проверил возвращаемое значение, забыл проверку на null, оставил кусок кода из отладки, забыл обработать исключительную ситуацию, забыл написать тесты и т. д.

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

Следует обращать внимание, что code review — это критика, а критика всегда обидна. Не нужно переходить на личности. Не нужно употреблять мат. Если вы видите ошибку, то не пишите, что этот код — говно. Лучше напишите как-то более мягко, например: «возможно, здесь стоит обработать NullPointerException.»

У меня на текущем месте работы есть правила, как правильно давать обратную связь о человеке, работе или чём-то ещё. Code Review — это и есть обратная связь. Больше всего из этих правил мне понравилось, что нельзя постоянно критиковать. Если ты указал на недостаток, то нужно обязательно найти что-то хорошее, а затем указать, что вот это место особенно понравилось. Смысл в том, что нельзя постоянно указывать ошибки и огрехи. Нужно обязательно похвалить за работу, найти что-то хорошее. До текущего места работы я даже как-то сам об этом не догадывался.

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

Не забывайте про то, что основная задача Code Review это:

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

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