Код ревью
В этом тексте я попытался сформулировать основные принципы хорошего, а главное, полезного код ревью. Я постарался сделать текст достаточно универсальным, чтобы применить к любой технологии и любой команде.
Цели ревью
Давайте определимся с целями код ревью.
- Проверка соответствия задаче
- Проверка логики написанного кода
- Проверка читаемости
- Проверка на необходимость рефакторинга
- Распространение знаний о коде
По пунктам:
Вносимые изменения решают какую-либо задачу (чинят баг, добавляют новую функциональность, улучшают читаемость кода и так далее). Задача читающего проверить, что код соответствует задаче.
Логика изменений должна быть понятна, и читающий должен иметь возможность проверить корректность в голове. Безусловно могут быть сложные задачи, в которых это сделать сложно или невозможно, но таких задач минимум.
Проверка читаемости нужна по той причине, что написанный однажды код придётся читать множество раз, и ревью — отличная возможность проверить, что сделать это легко.
Ревью предоставляет возможность получить обратную связь от других разработчиков. Это могут быть предложения переиспользовать уже готовый код, советы по улучшению текущего в соответствии с руководствами или лучшими практиками. В некотором роде про ревью можно думать как про ещё один уровень работы IDE, только с человеческим интеллектом.
Ревью также является механизмом, помогающим другим членам команды понимать проект, даже если они не занимаются им или занимаются другой его частью.
Очень важно понять, что в первую очередь ревью нужно тому, кто написал код, а не тому, кто его проверяет. Ревью — это способ улучшить свою квалификацию, обучаясь у других людей.
Не цели ревью
Целями не являются:
- Дискуссия о цели вносимых изменений
- Проверка синтаксиса на корректность.
- Организация спора про те или иные конструкции языка
Разберём по пунктам
На ревью не обсуждается нужно ли вносить предложенные изменения в целом. Необходимость изменений, а так же их дизайн обсуждается ещё до написания кода. Если это новые элементы интерфейса, то есть дизайн и мокапы. Если это устранения бага, то есть багрепорт. Если это рефакторинг, то есть задача, в которой обсуждается, почему рефакторинг необходим. На ревью не может быть ситуации, когда кто-то говорит, что фича не нужна.
Проверка синтаксиса на явные ошибки, а также кода на возможные ошибки должны производится исключительно автоматически. Это может быть компилятор, линтер, статический анализатор. Такие проверки могут выполняться локально или на отдельном сервере. Важно, чтобы инструментарий или договоренности не позволяли создавать ревью, в котором есть подобные ошибки.
Любые дискуссии на тему стиля кода и правил использования тех или иных конструкций языка должны быть полностью исключены из процесса ревью. Эти правила должны быть задокументированны отдельно и по возможности поддержаны инструментарием.
Как создавать ревью
Теперь, когда у нас перед глазами есть цели мы можем сформулировать некоторые базовые правила создания ревью.
В этом тексте я не акцентирую внимание на конкретном способе создания ревью, например Github pull request. Но все правила довольно общие и легко адаптируются к любому инструментарию.
Написание кода
Код ревью начинается с написания (изменения) кода. Эти изменения должны быть корректными, решающими конкретную задачу и написанными, согласно принятым в проекте стандартам.
Я не хочу углубляться в рассуждения о том, как правильно писать код и нужно ли ставить точку с запятой, но хочу отметить один момент: если у вас есть возможность написать код проще, не теряя в качестве, сделайте его проще. Помните, что пишите вы один раз, а читать будут много.
Разделение кода для ревью
Важный вопрос: какой размер кода оптимален для ревью? Ответ: чем меньше, тем лучше.
Зачастую бывает соблазн реализовать большую фичу и сделать огромный PR, после чего надеяться, что читающие смогут его осилить. На деле такие ревью обычно получают разрешение с пометкой “выглядит нормально” и отсутствием комментариев, что совсем не вяжется с задачей получить полезную обратную связь.
Как разбивать большой набор изменений на меньшие? Я попробую сформулировать ответ для разработки фронтенда.
- Отделяйте общие компоненты (такие как кнопка, панель, и так далее) в отдельный набор изменений.
- Отделяйте функции-утилиты в отдельный набор изменений.
- Разделяйте большие компоненты (такие как страницы) на внешние и внутренние части.
Другой вопрос: в каком порядке их сливать в основную ветку? Тут я видел два совершенно разных подхода.
Первый — это начинать с больших компонентов, например, фрейма страницы, а потом добавлять внутренние. Такой подход поможет читающим понять контекст каждого нового изменения.
Второй — это начинать с внутренних компонентов, делая их в изоляции и сопровождая примерами в виде storybook.
Оба принципа имеют свои плюсы и минусы, и сложно сказать, какой более удобен для читающих.
Важной частью эффективной работы с ревью является умение использовать инструменты. В случае с git необходимо понимание базовых операций, таких как commit, rebase, squash, reset, chunk и так далее. Зачастую GUI предоставляют интерфейс, достаточный для работы с ревью. Какой способ выбирать — решать вам.
Расскажу про свой подход. Обычно во время разработки новой фичи я использую отдельную ветку из мастера. Делаю коммиты много и часто. Когда фича готова, я делаю reset по отношению к мастер ветке. И после этого начинаю добавлять коммиты по одному, включая некоторое изменения. При этом каждый коммит атомарен и может быть отправлен на ревью сам по себе. Такой подход требует аккуратности, занимает время, и его нужно учитывать при оценке времени на реализацию фичи.
Оформление ревью
Изменения кода всегда должны быть в каком-то контексте. Проще всего этого достичь, приложив ссылку на задачу или документ. В случае, если задача потребовала нескольких наборов изменений (смотри предыдущую секцию), имеет смысл добавить ссылки на изменения, которые должны быть внесены до текущего. В случае с UI имеет смысл приложить к ревью скриншоты “до” и “после” или даже видео. Это позволит читающему ознакомиться с результатом и быстрее понять код.
Как читать ревью
Чтение ревью во многом обязанность, и с этим ничего не поделать. Но есть в ней и привилегии: возможность повлиять на качество общего кода, а так же научиться новому.
Относится к ревью чужого кода нужно так же серьёзно, как к написанию своего. Приняв изменения, вы тем самым подтверждаете, что считаете код правильным, а значит так же становитесь его владельцем и отвечаете за его поведение в продакшене. Подобную идею можно привить в команде на уровне культуры или даже формально (что, возможно, излишне).
Добавляйте комментарии, которые предполагают какое-либо действие:
- Если вы видите орфографическую ошибку, напишите правильный вариант.
- Если вам непонятна логика программы, то попросите её упростить или написать комментарий в коде.
- Если логика понятна, но кажется излишне сложной, предложите идею для улучшения.
В самом начале я уже упоминал, что на ревью не должно быть дискуссий на тему стиля кода. Если такое желание возникло, то дискуссию нужно выносить на отдельное обсуждение, после чего задокументировать и не возвращаться к вопросу.
Как вносить правки на ревью
Сам процесс зависит от используемого вами инструментария и договоренностей. При этом важно, чтобы и начальные изменения, и все правки были просмотрены проверяющими.
Ревью закончено, когда достигнут консенсус по всем замечаниям.
Бывает, что внести правки невозможно или очень дорого, но они не являются критичными. Например, сроки поджимают и нужно выпускать релиз. В таком случае выходом может быть создание отдельной задачи в трекере и TODO с номером задачи в коде.
Заключение
Код ревью — крайне ценный инструмент в процессе разработки ПО. При правильном подходе его можно сделать приятным и полезным для всей команды.
Не стоит относиться к этому тексту как к строгому руководству. Как и любые другие подобные размышления, стоит брать только то, что вы считаете обоснованным и пойдёт вам (вашей команде) на пользу.
