Руководства по код-ревью

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

Цели код-ревью

  • Повышение качества кода
  • Минимизация стоимости поддержки и развития проекта
  • Обучение и обмен знаниями в команде
  • Поддержание командной динамики и конструктивного взаимодействия

    Правила для авторов MR

  • Полностью понять МР за 10-15 минут. Размер MR не превышает 300-500 строк. Исключение составляют случаи, когда большая часть изменений — это статика (например, конфигурационные файлы или автогенерированный код).
  • Написаны тесты
  • Добавлены комментарии к сложным участкам кода
  • Описание MR понятно и структурировано

    Рекомендации для авторов MR

    Оптимальный размер MR

Почему важен небольшой MR? Качества ревью ухудшается. Количества принципиальных изменений становиться большим. Это приводит к увеличению когнитивной нагрузке. В результате, увеличивается шансы упустить что-то важное, а "мелочи" вообще игнорируются. В тяжелых МР-ах многие существенные вещи, котороые не желательно пропускать, становяться менее важными. При равных условиях, проекты где приемлемы огромные МР теряют в качестве.

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

Вдруг, обнаруживатеся, что что-то сделно не так или вообще не правильно, а МР очень большой и долго писался. Спустя продолжительное время выясняется, что время было потрачено не эффективно и много нужно переписать. Сроки завершения такого МР становяться не предсказуемыми. Чаще они сильно сдвигаются вправа. Это не единстванная проблем. Если сроки горят, то такой МР может заехать в мастер с очень большими архитектурными или другими сложностями. Это все может существанно повлиять на качества проекта и ухудшить его поддерживаемость. В конечном счете, нужно больше времени на создания новых фичей или изменения существующих.

Как добиться не больших МР? Залог правильного МР - правильная декомпозиция фичи/функциоанала. Не всегда можно сходу правильно определить границы контекста для каждой задачи. Если нет уверенносит, то на первой итерации, разбивается фича на большие задачи. Затем в процесс работы, когда проявляется больше контекста, нужно не забывать продолжать разбивать большую задачу на более маленькие. По итогу, получается список задач, у которых четко определены границы.

Одна задача - одна логическая единица, на выполнение которой нужно не более 500 строк кода. Хороший признак, что границы МР хорошо определены - легко и четко получаестя сформулировать описание МР.

Как подготовить качественный MR

  • Добавьте чёткое и ясное описание
  • Укажите цель и контекст изменений
  • Перечислите ключевые изменения
  • Поделитесь сомнениями или вопросами

    Черновые MR и предварительное ревью

    Если нет уверенности в решение. Нужно создать черновой МР, сформулировать вопросы и попросить коллег, провести предварительное ревью. Что бы ревьюверу было проще нужно подсветить проблему. Не обзязательно писать тесты и наводить порядок в коде. Это может оказаться пустой тратой времени. Важно написать решение и обсудить с коллегами. Когда будет уверенность, что в целом решение подходящее, тогда можно приступать к написанию тестов.

    Как реагировать на комментарии ревьюера

    Почему важно реагировать на комментарии? Обязательно реагируйте на комментарии. На все замечания нужна реакцию от владельца МР. Если ты согласен с замечанием, и обсуждать не чего, все-равно нужна реакция. Можно поставить знак согласия или любой другой знак в контексте сообщения. Подобное поведение снижает вероятность недоразумений. Ревьювер пишет комментарий для владельца МР, поэтому оставить такое сообщение без внимания может интерпритироваться очень по-разному. Так же у ревьювера могут возникать ощущения, того что он пишет комментарии вне куда, без обратной связи. Это может вызывать напряжение и не благоприятную рабочую атмосферу.

Почему важна оперативность? Скорость реакции во многих случаях сокращает время обсуждений. Интервьювер еще в процессе ревью, он не отвлекся на решение другой задачи и у него максимально возможное погружение в контекст. Это время самое конструктивное для обсуждений.

Как решать спорные вопросы? Если дисскуссия зашла в тупик, то собирай встречу или выноси дисскуссию в отдельный чат. Когда дисскусия переходи в спор, а аргументы уже закочились, то продолжать тред в том же духе мало эффективная стратегия. Взависимости от контекста, решить проблему можно несколькими способами:

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

    Правила для ревьюеров

  • Проверь бизнес-логику и архитектуру
  • Проверь все ли необходимое покрыто тестами
  • Проверь читаемость и поддерживаемость кода
  • Убедись, что замечания относятся к MR
  • Не затягивай время на ревью

    Рекомендации для ревьюеров

    Как писать конструктивные комментарии

    Комментарий должен быть четким и понятным. Нужно в одном сообщение сформулировать суть предложения, его обосновать и предложить варианты решений. Обязательно нужна причина следственная связь между тезисом(предложение) и обоснованием. Крайне не продуктивно использовать в качестве аргументов - тафтологию. Например, давайте используем паттерн проектирования X, потому что это правилильное решение, так пишут в умных книгах.

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

Оставлять комментарии, которые непосредственно относятся к МР. Если возникают идеи или предложения, не имеющие отношение к МР, то лучше их опубликовать в подходящем чате, что бы не задерживать работу над МР.

Как работать с большими MR

Если сталкиваешься с большим и сложным МР, нужно корректно запросить декомпозицию:

  • Объясните причину, почему его MR вызывает сложности
  • Предложи конкретные шаги для декомпозиции
  • Сформулируй, как разбивка улучшит процесс и поможет всей команде.

SLA для код-ревью

Цель:

  • Сократить время жизни MR
  • Повысить качество обратной связи и итогового кода
  • Улучшить командную динамику и ускорить выпуск фич.

Основные метрики SLA:

  • Время на первое ревью. Время, в течение которого ревьюер должен оставить первые комментарии после открытия MR.
    • Маленькие MR < 300 строк: 2-4 рабочих часа
    • Средние MR 300–500 строк: 1 рабочий день
    • Большие MR (> 500 строк): Сначала договоритесь с автором о возможной декомпозиции. Если разбить MR нельзя — сроки обсуждаются отдельно.
  • Общее время на ревью
    • Маленькие MR: до 1 рабочего дня.
    • Средние MR: до 2 рабочих дней.
    • Большие MR: согласование по времени отдельно с учётом сложности.

      Анти-паттерны в код-ревью