Code Review проводится в назначенных парах не мене 2 месяцев с даты формирования пары для лучшего понимания проекта поверяющими сторонами.
Таблица результатов ревью здесь.
Советы по Code Review:
Для всеx:
- Просматривайте только самое важное, дайте инструментам сделать все остальное
Вам не надо спорить о форматировании и кодстайле. Есть множество инструментов, которые последовательно решают эти вопросы. Важно, чтобы код был корректным, понятным и поддерживаемым. Конечно, стиль и форматирование — часть этого, но вы должны дать инструментам проверить эти вещи. - Каждый должен просматривать код
Некоторые люди в этом лучше других. Более опытный человек вполне может обнаружить больше ошибок, и это важно. Но более важным является поддержка позитивного отношения к проверке кода в целом, и это позволит избежать отношения «Мы против них», или того, что для ревью кода является обременительным для кого-то. - Просматривайте весь код
Нет кода слишком короткого или слишком простого. Если вы просматриваете всё, то ничего не останется пропущенным. Более того, это делает ревью частью процесса, привычкой, а не требованием. - Усвойте положительное отношение
Это важно как для ревьюверов, так и для авторов кода. Ревью кода — это не время чтобы получить все пятерки и влиять своим мастерством кодирования.
Вам не надо принимать оборонительную позицию. Подходите к ревью с позитивным настроем конструктивной критики, и вы сможете построить доверие к этому процессу.
Для ревьюверов:
- Ревью кода должно быть частым и короткими сессиями. Эффективность ваших ревью снижается примерно через час. Так что откладывание ревью, чтобы просмотреть их все в одной громадной сессии не поможет никому. Найдите время в течение дня, совпадающее с вашими перерывами — чтобы не нарушать состояние потока и сформировать привычку. Ваши коллеги будут благодарны вам за это. Ожидание может быть неприятно, и они могут решить вопросы быстрее, пока код еще свеж в их головах.
- Сказать «все хорошо» — это нормально. Не будьте придирчивы, вам не нужно искать проблему в каждом ревью.
- Используйте чеклист. Чеклисты для ревью кода обеспечивают согласованность — они дают убедиться, что каждый отслеживает важные и распространенные ошибки.
Для авторов кода:
- Код должен быть кратким. Через 200 строк кода эффективность кода значительно снижается. К тому времени, когда вы просмотрите 400 строк, они становятся почти бессмысленными.
- Обеспечьте контекст. Давайте ссылки на любые связанные тикеты или спецификации. Есть инструменты для ревью кода типа Kiln, которые помогут с этим. Давайте короткие, но полезные сообщения к коммитам и много комментариев в коде. Это поможет ревьюверу и вы получите меньше вопросов.
Обратите внимание на чек лист для code review, на данный момент это довольно общий список с которого мы начнем. В процессе работы мы будем его трасформировать и улучшать. Предложения по улучшению принимаются в письменном виде на почту и в общем чате. Чек лист нужно оптимизировать его под специфичные сценарии и проблемы. Отличный способ сделать это — отмечать вопросы, которые возникают в команде во время ревью кода в течение короткого времени. С этой информацией мы сможем найти самые распространенные ошибки, чтобы затем внести их в ваш чеклист.
Не бойтесь выбрасывать из чеклиста те элементы, которые нам не подходят (вы можете захотеть оставить редко встречающиеся, но все же критические пункты — например, связанные с безопасностью) Пишите о них немедля!
Начните использовать и держите в актуальном состоянии
Чек-лист CodeReview
Общее
- Работает ли код? Выполняет ли он свои прямые обязанности, корректна ли логика, и т. д.
- Легок ли код для понимания?
- Соответствует ли код вашему стилю написания кода? Обычно это относится к расположению скобок, названиям переменных и функций, длинам строк, отступам, форматированию и комментариям.
- Есть ли в ревью избыточный или повторяющийся код?
- Является ли код независимым, насколько это возможно?
- Можно ли избавиться от глобальных переменных или переместить их?
- Есть ли закомментированный код?
- У циклов есть установленная длина и корректные условия завершения?
- Может ли что-то в коде быть заменено библиотечными функциями?
- Может ли быть удалена часть кода, предназначенного для логгирования или отладки?
Безопасность
- Все ли входные данные проверяются (на корректный тип, длину, формат, диапазон) и кодируются?
- Обрабатываются ли ошибки при использовании сторонних утилит?
- Выходные данные проверяются и кодируются (прим. пер.: например, от XSS)?
- Обрабатываются ли неверные значения параметров?
Документация
- Есть ли комментарии? Раскрывают ли они смысл кода?
- Все ли функции прокомментированы?
- Есть ли какое-то необычное поведение или описание пограничных случаев?
- Использование и функционирование сторонних библиотек документировано?
- Все ли структуры данных и единицы измерения описаны?
- Есть ли незавершенный код? Если есть, должен ли он быть удален или помечен маркером типа «TODO»?
Тестирование
- Является ли код тестируемым? Например, он не должен содержать слишком много зависимостей или скрывать их, тестовые фреймворки должны иметь возможность использовать методы кода, и т. д.
- Есть ли тесты и если есть, то достаточны ли они? Например, они покрывают код в нужной мере.
- Юнит-тесты на самом деле проверяют, что код предоставляет требуемую функциональность?
- Все ли массивы проверяются на «выход за границы»?
- Может ли любой тестирующий код быть заменен с использованием существующего API?
Успехов!
