Правильный процесс ревью кода — это процесс контроля и итеративного улучшения качества продукта.
Контроля того:
1) что задача выполнена в полном объёме и соблюдены все критерии приемки.
2) что соблюдены общие правила и договорённости.
3) что решение не избыточно, его легко поддерживать и масштабировать в будущем.
Для начала будет хорошо задать своей команде такие вопросы:
Сколько времени занимает ревью кода для средней задачи?
Как у вас в команде понимают, что ревью задачи было сделано правильно?
Как вы пытаетесь уменьшить время ревью?
Командное владение кодом
Изменение фактора автобуса, за счет того, что каждый кусок в коде смотрела больше одной пары глаз, и знания не принадлежат одному конкретному члену команды.
Переиспользование кода
Когда несколько человек, решающих разные задачи и обладающие разными знаниями о продукте, смотрят код друг друга, то они могут заметить определенные закономерности и предложить переиспользовать 1 из уже готовых решений, либо вынести текущее в отдельный модуль/библиотеку.
Обмен знаниями внутри команды
У каждого в команде есть чему поучиться. Кто-то хорош в структурах данных и алгоритмах, другой разбирается в асинхронщине и базах данных, а кто-то мастер MVI и операционных систем. Возможность посмотреть на решения друг друга и задать вопросы это отличный способ поднять технический уровень всей команды.
Выявление ошибок
Наиболее очевидная цель review - это выявление ошибок разной степени критичности. Именно об этом в первую очередь думает ваш PM, когда вы предлагаете ему заложить определенный процент времени на code review.
Единообразие и простота проекта
Сколько есть на свете людей, столько и разных взглядов на то, как именно нужно писать код. Табы и пробелы, файловая структура проекта — при отсутствии консистентности это может сильно усложнить процесс чтения и анализа кода.
Рефлексия или self review
Большую часть ошибок в своем коде автор способен увидеть сам.
Поэтому перед тем, как подключать к ревью своих тиммейтов, стоит самостоятельно хоть раз внимательно просмотреть свой код.
Эта простая процедура поможет сильно сэкономить время и нервы, а также улучшит отношения с коллегами)
Скорость code review
Скорость команды в целом снижается. Да, если человек не отвечает быстро на код-ревью, он делает другую работу. Тем не менее, для остальной части команды новые функции и исправления ошибок задерживаются на дни, недели или месяцы, поскольку каждый PR ждёт код-ревью и повторного код-ревью.
Медленные ревью также препятствуют очистке кода, рефакторингу и дальнейшим улучшениям существующих PR.
Разработчики начинают протестовать против процесса код-ревью. Если рецензент отвечает только через несколько дней, но каждый раз запрашивает серьёзные изменения, это может быть неприятно и сложно для разработчиков. Часто это выражается в жалобах на то, насколько «строгим» является рецензент. Если рецензент запрашивает те же существенные изменения (изменения, которые действительно улучшают работоспособность кода), но каждый раз реагирует быстро, жалобы обычно исчезают. Большинство жалоб на процесс код-ревью на самом деле решаются путём ускорения процесса
Как быстро выполнять код-ревью?
Если вы не в середине сосредоточенной задачи, то должны сделать код-ревью вскоре после его поступления.
Один рабочий день — максимальное время для ответа (т. е. это одна из первых задач на следующее утро).
Следование этим рекомендациям означает, что типичный PR должен получить несколько раундов ревью (если это необходимо) в течение одного дня.
Скорость и отвлечения
Есть один момент, когда приоритет личной скорости превосходит командный. Если вы в середине сосредоточенной задачи, такой как написание кода, не отвлекайтесь на код-ревью. Есть исследования, о том что у разработчика может занять немало времени плавный вход в контекст, после отвлечения.Тем самым, отвлечение от кодинга на деле обходится команде не дешевле, чем просьба к другому разрабу подождать код-ревью.
Вместо этого дождитесь точки останова в своей работе. Например, после завершения текущей задачи, после обеда, возвращаясь со встречи, возвращаясь с офисной мини-кухни и т. д
Комментарии к PR
Будьте вежливым
Объясните свои рассуждения
Избегайте явных приказов, просто указывая на проблемы и позволяя разработчику решать
Поощряйте разработчиков упрощать код или добавлять комментарии вместо простых объяснений сложности
Хороший пример: Использование этой библиотеки в данном случае является лишним, так как она добавляет большое количество overhead'a, т.е функционала которым мы пока не пользуемся. Думаю в данном случае было бы целесобразно взять только нужную часть и использовать ее в новом модуле.
Плохой пример:
Зачем ты запилил эту либу? Откуда вообще нашел ее? Советую выпилить и самому сделать.
Чек-листы и их автоматизация (DevOps)
Примеры вопросов из такого чек-листа:
«Есть ли закомментированный код?»,
«Покрыта ли бизнес-логика тестами?»,
«Обрабатываются ли все ошибки?»,
«Вынесены ли строчки в константы?».
Запоминайте все вопросы, которые часто возникают у команды на review, и заносите их в список.
Главное — не давать этому чек-листу разрастаться. Думайте о способах автоматизации каждой из перечисленных там проверок — если приложить достаточно усилий, большую часть потребностей вы сможете закрыть. Есть огромное количество инструментов, которые могут помочь в этой работе — линтеры, статические анализаторы, детекторы дублирующегося кода
Например: SonarQube
Архитектурные встречи
Code Review — не самое подходящее место, чтобы спорить по поводу глобальных архитектурных решений. Код уже написан, решение выкачено, задача почти доделана, переписывать все с нуля — слишком затратно и зачастую не оправдано. Такие вещи нужно не оставлять на потом, а обсуждать заранее.
Как вариант — проведение архитектурной встречи, защиты и аргументации своего решения для команды, либо на словах, либо в виде схемы. Зачастую, бывают случаи, когда кого-то осеняет уже во время ревью — тогда это предложение, если оно действительно ценное, стоит задокументировать в issue-трекере и вернуться к нему по мере необходимости.
Видеть хорошее
Если видите что-то хорошее в PR, скажите разработчику, особенно когда он в лучшем виде решил проблему, изложенную в одном из ваших комментариев. Делая code review часто разработчики фокусируются только на ошибках, но они также должны поощрять и ценить хорошие практики. С точки зрения менторинга иногда даже более важно сказать , что именно человек сделал правильно, а не где ошибся
Нет идеального
Главным point'ом здесь является то, что не бывает «идеального» кода — бывает только код «получше». Не стоит требовать от автора полировать каждый крошечный кусок или фрагмент.
Скорее, рецензент должен сбалансировать необходимость дальнейшего прогресса по сравнению с важностью предлагаемых изменений. Вместо того, чтобы стремиться к идеалу, команда должна стремиться к непрерывному улучшению. Коммит, который в целом улучшает масштабируемость, читаемость и простоту системы, нельзя задерживать на дни или недели, потому что он не «идеален»
Ограничение размера code review
В среднем во всем всегда лучше найти золотую середину. На просторах интернета находится большое количество графиков соотношения размера PR к легкости его просматривания или количеству действительно найденных багов и тд. По опыту и наблюдениям из разных команд могу сказать что следует ограничить PR в своей команде до определенного количества изменений. Например 500, тем самым это поможет облегчить процесс просматривания PR и поможет членам команды научиться правильно дробить и декомпозировать задачу.
Работа по расписанию
Можно внедрить таблицу для code review по расписанию
Имя | 9-12 | 12-15 | 15-18 |
Алекс | + |
|
|
Дэвид |
| + |
|
Алеша |
|
| + |
Кто прав?
Когда разработчик не согласен с вашим комментарием, найдите время и рассмотрите его позицию. Зачастую он ближе вас к коду и поэтому у него действительно может быть более объективное представление. Есть ли смысл в его аргументе с точки зрения качества кода или бизнес решения? Если так, то признайте его правоту, и вопрос отпадёт
Однако разработчики не всегда правы. В этом случае тот кто проводит review должен объяснить, почему он считает, что его предложение является правильным. Хорошее объяснение демонстрирует как понимание ответа разработчика, так и дополнительную информацию о том, почему требуется изменение
В частности, когда рецензент считает, что его предложение улучшит качество кода, он должен настаивать на нём, если считает, что полученное улучшение качества оправдывает дополнительную работу. Улучшение качества кода, как правило, происходит небольшими шагами
Иногда требуется несколько раундов объяснений, прежде чем оно действительно будет принято. Просто убедитесь, что всегда остаётесь вежливым, и пусть разработчик знает, что вы его слышите, просто не согласны
Внутренние обиды
Рецензенты иногда считают, что разработчик расстроится, если рецензент настаивает на улучшении. Иногда разработчики действительно расстраиваются, но обычно ненадолго, а позже они будут очень благодарны, что вы помогли им улучшить качество их кода
Обычно, если вы вежливы в своих комментариях, разработчики на самом деле не расстраиваются вообще, и беспокойство находится только в голове рецензента
Расстраивает обычно стиль написания комментов , нежели настойчивость рецензента в отношении качества кода
На что стоит обратить внимание?
Насколько предлагаемое решение соответствует бизнес требованиям?
Насколько оно сложно для понимания, изменения, масштабирования?
Можно ли было сделать легче, понятнее, дешевле, быстрее и проще? Насколько получившаяся архитектура требовательна к ресурсам и какова сложность алгоритмов?
Сделано ли что-то лишнее, что не относится к задаче? Насколько это оправданно?
Используем ли мы внешние зависимости? Насколько это оправданно?
Имеет ли решение критические недостатки?
Опечатки в коде, magic numbers и другие неопределённые константы, которые разработик не учёл при решении задачи.
Потенциальные места где можно «выстрелить себе в ногу».
Тестируемость кода и наличие юнит-тестов. Данный этап проверок также очень важен, ведь он направлен на улучшение поддерживаемости кода.
Разработчик везде выбрал правильный naming?
Хорошие naming должен полностью передать то, чем является или что делает обьект, не будучи настолько длинным, что становится трудно читать.
