Всем привет! Меня зовут Владислав Шиханов, я ведущий программист в CDEK. В нашей компании работает 500+ IT-специалистов, именно мы создаём продукты и сервисы, из которых и состоит СДЭК. Моя команда разрабатывает сервисы для автоматизации процессов продаж и запуска новых продуктов.
Однажды мы столкнулись с тем, что процесс проверки кода начал задерживать выполнение задач и требовал слишком много усилий от участников. На ретро мы договорились повысить приоритет этого процесса и сделать доклад с best practices. В этой статье я решил поделиться главными выводами и лучшими практиками, которые помогли нам существенно упростить проверку кода.
На эту тему уже написано не мало статей — что‑то будет вам понятно и уже знакомо. Но я надеюсь, что в этой статье вы найдёте новое и полезное именно для себя.
В первую очередь материалы пригодятся разработчикам, потому что именно они выступают главными участниками code review. Также рекомендую прочитать статью менеджерам команд и тимлидам, чтобы корректно организовать процесс и повысить эффективность ваших команд при помощи предложенных практик. Текст не содержит сложной технической информации, не зависит от применяемого стека и будет полезен при любой квалификации.
Любите ли вы code review так, как люблю его я? Если нет — прочитайте статью. Если да, тоже прочитайте. Случаи успеха, равно как и провала использования практики проверки кода, предлагаю обсудить в комментариях.
Содержание
Цели и роли
Что такое code review?
Это систематический процесс, обеспечивающий контроль качества кода и обмен опытом внутри команды. Иными словами, это этап жизни задачи, когда разработчик реализовал задачу и даёт другим членам команды оценить то, что у него получилось. В свою очередь, другие разработчики отсматривают изменения в коде и находят места для улучшения. Таким образом, code review — это возможность совместить приятное с полезным: обменяться знаниями между участниками и улучшить продукт.
Чем не является проверка кода?
Совершенно точно можно сказать, что проверка — это не акция публичной порки и не возможность для самоутверждения за счёт менее опытных коллег. Это — систематический процесс, поэтому важно, чтобы он был приятным для всех участников. Иначе он либо заглохнет, либо сведётся к формальной процедуре: пришёл, поставил approve, ушёл. В этом случае код потеряет в качестве, а команда — в знаниях.
Что делает автор задачи?
Здесь всё довольно просто. До передачи задачи на проверку работу над ней нужно завершить — провести рефакторинг, чтобы код стал чистым и вызывал удовольствие при чтении, а также убедиться, что реализация работает. С последним отлично справляются написанные для задачи тесты и прекрасно дополняет ручное тестирование сценариев по задаче. Уместно вспомнить Роберта Мартина, который рекомендует писать код так, чтобы тестировщик не находил там ошибок. Это вольная трактовка, но идея не в том, чтобы стремиться спрятать баги, а в том, что нужно их отловить самостоятельно ещё до передачи задачи в тест.
Здесь же будет уместно вспомнить о метрике WTF в минуту. Чем меньше WTF вызывает у проверяющего код, тем легче и приятнее ему делать свою работу. Поэтому до приглашения коллег на review стоит свежим взглядом отсмотреть изменения, сделанные в задаче. Я называю этот процесс self‑review. Статические анализаторы кода и шаблон форматирования для IDE приближают стиль кода к стандартам компании и упрощают проверку. В идеале, статический анализ встраивается в CI, но если это пока не сделано, то запускайте анализатор вручную до передачи задачи коллегам.
Иногда можно получить замечание за чужой код, когда в поле зрения проверяющего попал старый и не слишком удачно написанный класс или метод. Можно сыграть на опережение и следовать правилу бойскаута: оставлять код в лучшем состоянии, чем он был до.
Что делает проверяющий?
Проверяющему нужно погрузиться в контекст задачи и прочесть код. Результат review — обратная связь. В своей практике я встречал одну рекомендацию — сначала отмечать положительные моменты, а затем переходить к местам, которые стоит улучшить. Можно описать впечатление от реализации задачи, отметить интересные решения или технологии, которые удалось применить. Далее можно поделиться рекомендациями по улучшению кода. Их нужно давать беспристрастно, доброжелательно и аргументированно. В результате обратная связь будет восприниматься не как замечания, а как возможности для совершенствования.
Хорошие практики
Проверка кода существует со времён перфокарт. С тех пор в сообществе были сформулированы рецепты успешного code review, которые я собрал воедино и делюсь здесь. Напишите в комментариях, какие ещё практики считаются «must have» в вашей команде.
Использовать чек-листы
Ваш процесс code review должен быть прозрачным и симметричным для всех участников, поэтому важно иметь правила игры, известные и понятные каждому члену команды.
Как правило, в компании, где вы работаете, существуют требования к стилю и качеству кода. Также они могут встречаться и внутри каждой отдельно взятой команды. В идеале, эти стандарты должны быть агрегированы вами и вашими коллегами вместе, в том числе с учётом стандартов вашей организации. Этот набор может выступать своего рода чек‑листом, на который можно ссылаться любому члену команды в случаях, когда в коде требуются правки. Так комментарии приобретают аргументированность и процесс проверки становится упорядоченным.
Декомпозировать задачи
Проверка кода требует концентрации и внимания, поэтому качественно проверить огромные задачи очень сложно. В результате процесс может либо скатиться к чтению кода по диагонали, либо к откладыванию code review в долгий ящик, задержке выпуска задачи, а после — к чтению по диагонали.
Поэтому важно декомпозировать задачи так, чтобы число изменённых файлов и строк было небольшим. Тогда code review будет подробным и быстрым. В разных статьях я встречал рекомендацию, что число изменённых строк должно быть не более 400, а коллеги по компании рекомендуют до 100, а лучше до 50 строк в одном MR. Важно оговориться, что эти числа достаточно условные, а реализовать изменение атомарно не всегда возможно. Тем не менее, это важный принцип — никто не захочет незамедлительно проверять целый проект.
Во время проверки делайте паузы. Объём прочитанного за раз кода также влияет на результат. При длительном чтении теряется концентрация, поэтому, по возможности, не стоит проверять задачу более одного часа подряд.
Аннотировать ключевые моменты
Чем легче будет проверяющему погрузиться в контекст задачи, тем быстрее он приступит к проверке. Для этого можно описать, что было сделано, какие решения и по какой причине применены, на что следует обратить внимание.
Пример:
Расставлять приоритеты
Code review — операция, которая блокирует выход задачи, поэтому желательно проводить её как можно раньше. Выработайте привычку проверять код в начале или конце рабочего дня, либо при смене рабочих контекстов. При этом важно не вызывать у членов команды дополнительных смен контекстов и просить проверить только срочные задачи.
Разделять на раунды
Мелкие замечания потеряют актуальность после исправления более крупных и верхнеуровневых, поэтому разделяйте code review на раунды. Рассматривайте сначала как спроектирована задача, а уже после переходите к реализации и финализируйте мелочами типа стиля кода. Таким образом на проверку останутся только актуальные замечания, что снизит психологическое давление на автора задачи.
Коллега поделился примером из жизни:
«Однажды падаван — начинающий мидл — самостоятельно и безнадзорно кодил неделю, пока я был в отпуске. После этого проверял ещё неделю в несколько шагов: сначала находил крупные концептуальные ошибки, потом помельче, и в конце уже занялся шлифовкой стиля кода».
В идеале процессы должны выстраиваться так, чтобы переделывать всё не приходилось. Например, вы можете совместно с кем‑то из команды проектировать задачу до реализации и использовать парное программирование в процессе. Итеративный подход упростит внесение изменений в не слишком удачный код и, надеюсь, сохранит психическое здоровье участникам.
Достаточно поднять оценку на один балл
Не нужно стремиться за один сеанс code review создать идеального программиста.
Обилие замечаний давит на автора задачи — эта практика развивает идею о раундах. Никто не рождается с умением писать совершенный код. Мы учимся писать качественно постепенно, поэтому будет нечестно требовать от кого бы то ни было делать это классно и чисто, знать все лучшие практики и стандарты. Итеративно повышая качество кода благодаря проверкам, мы меньше загружаем автора задачи, позволяя ему эффективно усваивать и применять новые знания в будущем.
Иными словами, если на review приходит код, написанный на условную двойку, не нужно заставлять автора сразу же написать код на пятёрку. В рамках одной проверки достаточно хорошим результатом будет поднятие оценки всего на один балл, чтобы решение задачи было просто удовлетворительным. Затем на review будут приходить работы, сделанные на тройку, четвёрку и, наконец, на пятёрку.
Обратный случай из практики коллег:
«Первые пара проверок обычно происходят жуть и ужас. Они могут напоминать избиения палками. Потом человек понимает, чего от него хотят, и довольно быстро переходит на командный стиль написания кода. Дальше бОльшая часть проверок проходит уже в стиле «докопаться до орфографии», ну или найти косяк, который пропущен по невнимательности. Итеративной проверкой было бы «накидать не больше N замечаний на один MR». Сталкивался с подобным подходом, но не могу сказать, что это идеальный вариант. Психологически, наверное, кому‑то проще, но если грубых ошибок не N, а N+1, то будет проблема».
Думаю, что стоит искать баланс: в прод не должен уходить код с грубыми ошибками, при этом, если таких ошибок много, то на негрубые можно закрыть глаза и отложить до следующей проверки.
Не давать однотипные отзывы
Если допущена однотипная ошибка — не стоит захламлять весь MR однотипными комментариями. Достаточно указать на неё и попросить проверить остальные места в коде. Все мы знаем принцип DRY, почему бы не применить его и в code review?
Комментировать аргументированно и не повелительно
Обращение в форме приказа вызывает отторжение и конфронтацию. Если существует необходимость в исправлении, лучше формулировать её в формате предложения или просьбы. При этом не стоит забывать об аргументации.
Используя обезличенные формы общения, мы критикуем код, но не разработчика. Соответственно, критика воспринимается менее остро. На опечатку можно указать через стрелочку: слева — исходный вариант, справа — исправленный. Для более сложных случаев стоит оставить в комментарии фрагмент улучшенного кода и краткое описание его преимуществ. Местоимение «мы» в комментариях фокусирует внимание на том, что мы действуем как команда.
Например:
Approve – когда критичных замечаний нет
Как уже говорилось выше, code review — процесс блокирующий, поэтому важно не затягивать с одобрением задачи, если остались мелкие правки. Автор задачи может выполнить их и без надзора, а одобрение merge request'а поднимет его моральный дух. Таким образом мы уменьшаем блокирование задачи, выражаем доверие членам команды и бережём своё время.
Обозначать важность
Часто в коде есть недоработки, которые не мешают жить, но без них было бы лучше. В нашей команде мы начинаем такие комментарии со слова nit, обозначая минорность замечания. Это может быть любое замечание, не влияющее на функциональность и поддерживаемость кода, но при этом требующее внимания автора задачи.
Итоги
Code review станет хорошей практикой, если будет приносить удовольствие всем участникам. Тогда этот процесс будет способствовать повышению качества кода и обмену знаниями.
Автор задачи должен подготовить код к проверке: написать его чисто, настроить тесты, проверить при помощи статического анализатора, привести форматирование к стандарту компании при помощи IDE и файла форматирования. И проверить свои изменения. По возможности следует не делать задачу излишне большой и кратко описывать ключевые моменты, которые были реализованы. Тогда проверяющему будет проще погрузиться в контекст и обратить внимание на нюансы.
Важно не воспринимать замечания на свой счёт. Все мы тут, конечно, творцы, но также мы люди, а значит, совершаем ошибки.
Проверяющему важно проводить review в срок: выделять необходимое время, погружаться в контекст. Замечания стоит описывать обезличенно, аргументированно, доброжелательно и не повелительно. В этом случае автор с большей вероятностью адекватно воспримет критику и в следующий раз реализует задачу лучше.
Не стоит требовать идеального исполнения — достаточно каждый раз делать работающий код немного лучше. Оставлять замечания следует итеративно. Сначала обращайте внимание на высокоуровневые проблемы, а уже после исправления, по необходимости, дорабатывайте более мелкие детали.
Рекомендую проверять план решения задачи до начала реализации. Это позволит получить спроектированное решение, которое уже согласовано с членами команды, что сократит число серьёзных замечаний по результатам реализации.
Каких практик, на ваш взгляд, не хватило в этой статье? Буду рад прочитать о них в комментариях, равно как и об успешных и неуспешных историях их применения.
Желаю всем нам исключительно приятных проверок кода, качественных задач и полезных замечаний.
Другие материалы
Code review по-человечески — перевод отличной статьи о code review с близким списком практик и примерами их использования
How to Make Your Code Reviewer Fall in Love with You — оригинал другой статьи того же автора с мыслями о том, как можно улучшить процесс review со стороны автора задачи
Java code review checklist — пример чек-листа для review, который можно адаптировать для использования в своей команде
Unlearning toxic behaviours in a code review culture — статья с практиками по повышению культуры code review
Ship / Show / Ask — любопытное предложение по проведению проверки кода после его вливания в основную ветвь
pixsel
Понравилась общая концепция, особенно принцип того, что сначала акцентировать внимание на ошибках высокоуровневых критичных, а потом уже стиль и оформление кода.
vladislav-shikhanov Автор
Спасибо!