Очень важно уделять процессу code review достаточно времени, поскольку это может сэкономить очень много времени в будущем.
Основная цель code review - не допустить плохого кода в основной ветки нашего репозитория. Также code review помогает исключить некоторые очевидные баги, хотя это и не основная цель проверки кода. Например:
- Разработчики мотивированны писать хороший код, поскольку их код в любом случае обязан будет пройти Code Review и плохой код в большинстве случаев его не пройдет
- Разработчикам полезно делиться знаниями друг с другом. В процессе code review это можно делать достаточно просто, ведь у вас есть возможность подсмотреть интересное решение у вашего коллеги, либо подсказать ему что нибудь.
- Консистенция кодовой базы позволяет разработчикам в команде говорить на одном языке и понимать любой код в рамках большого проекта, помогает избежать багов из-за неочевидных решений
- Автору кода трудно оценить свой код, в то время как ревьюер может с легкостью заметить сложный(плохой) код и попросить его поправить. Также ревьюер может заметить некоторые ошибки или баги, которые пропустил реализатор задачи
Для создания качественного и удобного для проверки MR нужно выполнить несколько простых действий, которые сэкономят время ревьюеру.
MR должен быть оформлен в соответствии со следующими правилами:
- Ссылка на задачу должна быть прикреплена
- Краткое описание задачи и выполненной работы
- Скриншоты UI(если есть) до и после изменений
- Информация по покрытию кода тестами(скриншот coverage)
- MR не должен содержать в себе более 20 измененных файлов, иначе проверять его будет очень сложно
- MR должен пройти все проверки CI(тесты, линтер и т.д.)
- Информации о тестировании на различных устройствах/браузерах(если задача этого требует)
Если у вас есть сомнения насчет какого-либо решения в вашем MR, то вам нужно об этом обязательно написать в описании, а также продублировать в виде комментария для строчки кода с сомнительным решением. Если вы считаете, что ваше решение вполне нормальное, а ревьюер просит вас его переделать - не стесняйтесь выяснять причины и просить его показать на примере чего он хочет от вас добиться.
Если вы чувствуете, что ваш MR будет слишком большим, позаботьтесь заранее о том, чтобы разделить его на несколько логических MR. Это можно сделать при помощи добавления Feature Flags. Пример:
- Антон верстает кейс, в котором добавляется много новых компонентов(30+ изменений файлов)
- Чтобы упростить задачу проверяющим, Антон создает первый MR с добавлением пустой страницы и включением feature flag для отключения этой страницы в продакшене
- Далее Антон работает и порционно поставляет изменения для страницы кейса(например, по секциям)
- В финальном MR Антон выключает feature flag
Есть также альтернативный способ решения этой проблемы. Вам нужно создать основную ветку с фичей и создавать небольшие MR в нее. После окончания работы нужно создать МР из ветки с фичей в основную ветку и прикрепить в описании все ссылки на предыдущие МР, в которых был проревьюен текущий код.
Ваша цель, в первую очередь, подтвердить, что код соответствует стандартам качества в вашем проекте и отловить потенциальные проблемы или плохой код.
Важно! Не стоит проверять MR, который не проходит CI проверки. Код в этом MR скорее всего поменяется и проверять придется заново
Из описания MR вам должно быть четко понятно, что делает код, который находится в данном MR. Если описание не достаточно развернутое - просите автора MR дополнить его. Также следует детально изучить задачу, код который вы собираетесь ревьюить
В первую очередь необходимо запустить код у себя на машине и проверить работоспособность задачи. Если у вас возникнут вопросы и проблемы по выполненной задаче, нужно отписать об этом в общих комментариях к MR. Это нужно для того, чтобы разработчики имели дополнительный слой проверки и баги отлавливались до их попадания на staging.
Это может быть достаточно затратно по времени, но не ленитесь и всегда проверяйте работоспособность кода локально!
Проверьте код концептуально, расположение модулей(файлов), наименование, правильное использование различных сущностей
Проверьте детали реализации бизнес логики
Проверьте наличие обработки всех исключений, которые могут произойти в процессе работы кода. Все исключения должны быть обработаны
Если вы увидели сложный участок в коде, то попросите разработчика попытаться от него избавиться. Если не удается этого сделать, то нужно пометить этот код комментарием с развернутым описанием проблемы и причины переусложнений
Проверьте наличие достаточного количества тестов для написанного кода. Coverage не должен падать больше чем на 0.5%
Прежде чем написать комментарий, удостоверьтесь, что вы правильно поняли суть проблемы. В идеале, вы должны предложить решение, которое эту проблему исправит. Пример идеального комментария:
После того, как вы заапрувили MR, отпишите об этом в комментариях к нему. Поскольку в данный момент в нашей команде работает 4 разработчика, хватит 2 из 3 аппрувов, чтобы MR был готов к мержу.