1. Имеют ли смысл изменения?
2. Насколько хорошо и понятно составлено описание?
3. Какая бизнес и/или техническая цель изменений?

Перед началом ревью

Общий взгляд на изменения

• Проверьте название PR и его описание
• Определите общую идею изменений, их дизайн и функциональность

Определите основные части изменений

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

Проверяйте логически связанные части

• Проверьте реализацию ключевых компонент
• Проверьте, как ключевые части взаимосвязаны
• Покрыт ли новый функционал тестами?
• Насколько хорошо читается новый код?

Дизайн изменений

• Имеет ли смысл взаимодействие различных частей кода в PR?
• Относится ли изменение к вашей кодовой базе?
• Хорошо ли он интегрируется с остальной частью системы?

• Стоит ли добавить эту функциональность именно сейчас?

Функциональность

• Функционал делает то, что задумал разработчик?

• Реализация функционала полезна для пользователей? («Пользователи» - это, как правило, как конечные пользователи, когда на них повлияют изменения, так и разработчики, которым придется «использовать» этот код в будущем)

• Не ведется ли параллельно разработка того, что будет конфликтовать с текущим PR?

Сложность

• Изменения выглядят сложнее, чем они должны быть?
• Автор добавил избыточную функциональность?
• Встречается чрезмерное проектирование?
• Код непонятен?

«Слишком сложный»

обычно означает, что «код не может быть быстро прочитан и понят»

https://en.wikipedia.org/wiki/Brian_Kernighan

Чрезмерное проектирование

- это реализация более универсального кода, чем нужно, или добавленная функциональность, которая на данный момент системе не нужна

Тесты

• Проверьте наличие юнит и интеграционных тестов, в зависимости от изменений
• Тесты должны быть в том же PR, что и измененный код
• Тесты не должны тестировать сами себя
• Тесты должны описывать бизнес логику
• Тесты должны покрывать конкретные части функционала

https://amway-prod.tt.com.pl/confluence/pages/viewpage.action?pageId=20386424

Нейминг

Насколько информативны имена переменных, методов и т.д.?

Хорошее имя

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

Комментарии в коде

В комментариях нет необходимости, если код простой и самодокументируемый

(но так происходит не всегда)

Внимание к комментариям

Оставил ли разработчик комментарии к сложным участкам кода? Если да, то это повод задуматься над таким участком. Возможно у того, кто делает ревью, появится идея, как сделать проще.

Стиль

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

https://amway-prod.tt.com.pl/confluence/display/AMWEIA/UI+Dev+JS+Code+style+guide

Документация

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

Чтение кода

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

• Если вы понимаете код, но не чувствуете себя достаточно компетентным для проверки какой-либо части, то пригласите в PR проверяющего, кто обладает соответствующей квалификацией

Замечать хорошее

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

Итого

Делая код ревью, убедитесь, что:

• Код хорошо спроектирован

• Функциональность действительно необходима пользователям кода и приложения

• Любые изменения в интерфейсе разумны и выглядят хорошо

• Любое асинхронное программирование выполняется в правильном порядке и не порождает необработанных исключений

• Код не сложнее, чем должен быть

• Разработчик не реализует вещи наперёд без четкого понимания и требований к их использованию

• Код покрыт тестами

• Тесты хорошо продуманы, корректны и полезны

• Разработчик использовал подходящие имена для всего

• Комментарии понятны и полезны, и в основном объясняют причину, а не то, что сделано

• Код надлежащим образом документирован

• Код соответствует нашим руководствам по стилю

Помните о том, что

• Код ревью (и тесты) являются частью definition of done и к ним надо относиться серьёзно

• Умение делать ревью такой же навык, как и написание кода и поддаётся развитию  но только если вы этим будете это делать