Code review может быть большой болью для команды, которая только начинает его внедрять. Вы в любом случае наступите на много граблей: будете проводить ревью дольше, чем пишете код, устраивать смертельные споры про расположение скобочек и разбираться, можно ли сливать ветку в master до аппрува команды или нет. Я собрал ряд практик, которые помогут вам сделать процесс адаптации чуть менее болезненным — по крайней мере, мне они точно помогли.
Этот материал — краткая выжимка моего опыта, накопленного за несколько лет работы в крупных командах мобильной разработки. Опыт по большей части в мобильной разработке, что оказало влияние на используемые примеры и ссылки. Для тех, кто предпочитает не читать, а смотреть, в течение пары месяцев должно появиться видео с конференции Mobius, где я рассказываю доклад на эту же тему, но с кучей подробных практических примеров.
Цели Code Review
Для начала тезисно обозначу цели, которые стоит преследовать при проведении инспекций кода. Именно этими целями и их приоритетом относительно друг друга я и руководствуюсь, советуя внедрять ту или иную практику.
- Коллективное владение кодом
Уменьшение фактора автобуса за счет того, что каждый компонент в коде видела и анализировала больше, чем одна пара глаз, и знания не принадлежат одному конкретному разработчику. - Переиспользование кода
Когда несколько человек, решающих разные рабочие задачи и обладающие разным набором знаний о продукте, смотрят код друг друга — они могут увидеть определенные закономерности и либо предложить переиспользовать одно из уже готовых решений, либо выделить текущее в отдельную библиотеку. - Обмен знаниями в команде
У каждого в команде есть чему поучиться. Кто-то хорош в алгоритмах, кто-то разбирается в метапрограммировании, а кто-то джедай MVC. Возможность посмотреть на решения друг друга и задать вопросы — отличный способ поднять технический уровень всей команды. - Обнаружение ошибок
Наиболее очевидная цель — обнаружение ошибок разной степени критичности. Именно об этом в первую очередь думает ваш менеджер, когда вы предлагаете ему заложить определенный процесс времени на code review. - Единообразие проекта
Сколько есть на свете программистов, столько и разных взглядов на то, как именно нужно писать код. Табы и пробелы, файловая структура проекта — при отсутствии консистентности это может сильно усложнить процесс чтения и анализа кода.
Ну а теперь — к делу.
Модели организации Code Review в разных командах
Команды бывают разные — где-то в одном отделе сидит тридцать человек, которые разрабатывают один проект, а где-то разработчик в одиночку поддерживает сразу десяток приложений.
Небольшая команда
Первый случай, наверное, самый часто встречающийся. Это команда из нескольких человек — скажем, до четырех, которые работают над одним проектом. Здесь уже обычно вовсю расцветает Git Flow, задачи разрабатываются независимо друг от друга в отдельных ветках, а по окончании сливаются в develop.
В таком случае лучше всего взлетает модель, при которой для слияния ветки с задачей в основную рабочую ветку требуется одобрение всех участников команды. Это помогает получить максимальные преимущества по всем перечисленным в первом разделе целям проведения ревью.
Несколько отдельных команд
Масштабируем предыдущую ситуацию и представим крупного аутсорсера, в которого входят несколько таких маленьких команд, каждая из которых работает над своим собственным проектом. В таких ситуациях иерархическая структура обычно усложняется и, помимо наличия ведущих разработчиков в каждой из команд, добавляется новая прослойка, тимлид или техлид, который отвечает сразу за несколько таких проектов.
Основной подход остается неизменным — на все новые задачи создаются ревью, в которых принимает участие вся проектная команда, но итоговую ответственность за принятые технические решения несет ведущий разработчик. Кроме того, к важным ревью нужно подключать еще и лида — таким образом он будет оставаться в курсе дел, происходящих на проекте и в нужный момент подключаться к решению возникающих проблем.
В нагрузку идет еще одна дополнительная активность — очень эффективными показывают себя периодические кросс-проектные ревью, когда разработчик знакомится с остальными проектами. Это позволяет каждому быть в общих чертах в курсе того, где какие технологии и подходы используются, знать, кто и в чем разбирается. Подходить к кросс-проектным ревью можно по-разному — вести сквозной перечень всех проектов и сортировать его по последнему ревью, или ставить каждому разработчику цель — раз в определенный промежуток времени инспектировать все остальные проекты. Такие ревью проводятся в основном в ознакомительных целях — поэтому обычно одобрения стороннего ревьюера не требуется для того, чтобы завершить задачу — человек, который смотрит код, перенимает опыт у других команд.
Большая команда
Еще одна ситуация — большая команда, работающая над одним крупным проектом. В такой ситуации хорошо оправдывает себя установление определенного порога ревьюеров, после одобрения которых можно сливать ветку. Это значение может зависеть от размера команды, но разумные рамки — 2-4 человека.
Большим вопросом остается определение конкретных ревьюеров — они могут выбираться как рандомно, кто первый откликнется, так и специально автором ревью. Можно автоматизировать этот процесс, чтобы избежать споров и быть более объективными. Как пример — определять владельца кода по данным истории системы контроля версий или системы для code review.
Разработчик-одиночка
Последний случай — самый грустный. Разработчик работает один в команде. Во-первых, сразу отпадает такая цель, как коллективное владение кодом — нет коллектива, нет и владения. Остальные цели тоже становятся довольно вырожденными — но, тем не менее, сторонний взгляд на код поможет найти часть ошибок и повысить технический уровень в целом.
В таком случае разработчику нужно обращаться за помощью в открытое сообщество. Я советую три канала.
- Различные чаты и сообщества разработчиков
Два моих любимых — Slack-чат Cocoa Developers Club и Telegram-чат iOS Good Talks. - Встречи разработчиков
Например, Peer Lab, который проводится в Москве каждую неделю. - Совместная работа с коллегой из другой функции
Можно подключить своего коллегу, который занимается разработкой под другую платформу — Android, Web или Backend. Но будьте осторожными — не все готовы и хотят погружаться в контекст проблем из другой области разработки.
Практики проведения Code Review
Работа по расписанию
Основная проблема заключается в таком понятии, как состояние потока, из которого вырывать разработчика — смерти подобно. Сами представьте ситуацию: вы с головой погрузились в разработку своей фичи, строите в голове дворцы абстракций, и неожиданно за плечо робко трогает коллега и просит взглянуть на его код. Ничего хорошего из этого не выйдет — разработчик и своей задачей не может больше нормально заниматься, и для ревью слишком предвзят и раздражен.
Решает ситуацию расписание. Каждый разработчик в команде определяет промежуток дня, в который он будет смотреть ревью. Расписание всей команды можно держать в одной таблице, чтобы, ориентируясь на это, можно было подбирать себе ревьюеров или рассчитывать время проверки задачи. Я, к примеру, всегда выделял час времени с утра, сразу после прихода на работу — как раз помогало войти в рабочий ритм.
Конечно, есть случаи, когда этот механизм не работает — некоторые ревью действительно нужно смотреть максимально быстро. Главное — придерживаться такого режима, чтобы возникновение подобных ситуаций не становилось привычкой.
Архитектурные review
Я уверен, что вы сталкивались с ситуацией, когда после реализации задачи выясняется, что нужно было ее сделать абсолютно по-другому. Code Review — не самое подходящее место, чтобы спорить по поводу глобальных архитектурных решений. Код уже написан, задача почти доделана, переписывать все с нуля — слишком затратно. Такие вещи нужно не оставлять на потом, а обсуждать заранее.
Как вариант — проведение архитектурного review, защиты своего компонента перед командой либо на словах, либо в виде схемы. Конечно, бывают случаи, когда кого-то осеняет уже во время ревью — тогда это предложение, если оно действительно ценное, стоит задокументировать в issue-трекере и никогда к нему больше не возвращаться.
В качестве участников можно привлекать технического лидера проекта, разработчиков, непосредственно заинтересованных в реализации этой задачи, и просто всех желающих из команды.
Ограничение размера review
Есть один замечательный парадокс — чем меньше кода на review, тем больше к нему оставляется комментариев. Это правило остается неизменным практически в любой команде. Для нескольких строк кода характерно наличие огромных и развесистых дискуссий. При увеличении объема изменений до нескольких тысяч строк количество комментариев резко падает до отметки одного-двух, на которой и остается.
Эта практика очень простая — нельзя отправлять на ревью слишком много изменений за раз. Это требование вполне коррелирует со стандартными правилами по декомпозиции задач, каждая из которых не должна превышать рабочего дня. Если залить слишком много кода, то у ревьюеров очень быстро замылится глаз — и многие проблемы будут просто пропускаться. Кстати, сюда же можно отнести и дополнительное требование — убирайте весь информационный шум, который мешает ревьюерам сосредоточиться на фактическом смысле изменений.
Self review
Большую часть фактических ошибок в своем коде автор способен увидеть сам. Поэтому перед тем, как подключать к ревью своих коллег, стоит самостоятельно один или два раза внимательно просмотреть свой код. Эта несложная процедура поможет очень сильно сэкономить время и нервы и улучшить отношения с коллегами.
Описание review
Когда проводишь code review на постоянной основе, сложно фокусироваться на фактическом смысле изменений — об этом я уже упоминал. Еще один способ помочь ревьюерам удерживать фокус — детально описывать смысл изменений. Обычно я работаю по следующему шаблону:
- Название review
- Краткое описание изменений
- Вещи, требующие особого внимания
- Связанные задачи и материалы
Чек-листы и их автоматизация
Чек-листы — это круто. Используйте их и для code review. Примеры вопросов из такого чек-листа: «Есть ли закомментированный код?», «Покрыта ли бизнес-логика тестами?», «Обрабатываются ли все ошибки?», «Вынесены ли строчки в константы?». Запоминайте все вопросы, которые часто возникают у команды на review, и заносите их в список.
Главное — не давать этому чек-листу разрастаться. Думайте о способах автоматизации каждой из перечисленных там проверок — если приложить достаточно усилий, большую часть потребностей вы сможете закрыть. Есть огромное количество инструментов, которые могут помочь в этой работе — линтеры, статические анализаторы, детекторы дублирующегося кода.
Измерение эффективности Code Review
Внедрение любого процесса должно сопровождаться сбором метрик его эффективности и их дальнейшим анализом. Сам процесс — не самоцель. Code review — не исключение, важно уметь понять, насколько сильно он помогает команде в ее работе.
Удобный вариант — исходить из целей, которых мы хотим достичь при внедрении code review. В качестве шпаргалки можно использовать те, которые я обозначил в первом разделе. Один из способов измерения эффективности достижения таких целей — использование подхода Goal-Question-Metric. Состоит он из трех этапов — определение цели, которую мы хотим измерить, постановка вопросов, которые помогут понять уровень достижения цели, и определение метрик, которые помогают ответить на поставленные вопросы. Разберем на примере.
Сначала нужно определить намеченную цель. Как пример — «Повысить уровень переиспользования кода». Можно и даже нужно использовать сразу нескольких целей — и измерять их независимо друг от друга.
Затем сформулируем вопросы, на которые нужно ответить, чтобы понять, достигаем ли мы цели. В нашем случае это «Используем ли мы общие библиотеки в проекте?», «Вынесены ли визуальные стили в отдельный компонент?», «Много ли кода дублируется?».
Ну и последнее — определение метрик, которые помогут ответить на вопрос. В нашем случае это количество общих библиотек, количество мест, где остались захардкоженные стили, уровень дублирования кода.
Использовать фреймворк довольно просто. Измеряем обозначенные метрики до внедрения процесса, определяем желаемый результат, и периодически, раз в месяц или в квартал, повторяем эти измерения. Если их можно автоматизировать — еще круче. Спустя какое-то время анализируем результат и определяем, достаточно ли изменились метрики, чтобы счесть процесс эффективным.
Почему эта метрика действительно хороша? Представьте, что большую часть времени разработчики занимались не предложениями по переиспользованию кода, а холиварами на тему пробелов или табов — с такими ревью эти ключевые показатели не сильно бы выросли, нужного результата мы бы не достигли и поняли бы, что что-то с процессом не то.
Кроме того, полезно собирать еще некоторые метрики, которые могут подсказать, с какими проблемами при использовании процесса мы столкнулись.
- Inspection rate — скорость проведения review,
- Defect rate — количество найденных проблем на час времени, потраченного на review,
- Defect density — количество найденных проблем на строчку кода.
Если собирать такие данные по каждому ревью, то можно вполне адекватно оценить уровень вовлеченности в процесс у каждого участника команды. Ряд систем для проведения code review автоматически собирает такие данные — например, Crucible от Atlassian.
Заключение
Как я уже упоминал во введении, эта статья — краткая выжимка моего выступления, в котором я затрагиваю еще и различные инструменты для автоматизации процесса, правила и нормы поведения в команде и прочие аспекты проведения code review. Чтобы не пропустить появление записи — подписывайтесь на мой Telegram-канал, куда я обязательно выложу его по готовности. Если у вас остались вопросы — приходите на наш митап в субботу, 17 июня, и обсудим их в неформальной обстановке.
Полезные ссылки
- Примеры проведения хороших review: Wire, Artsy, Kickstarter.
- Slack-чат Cocoa Developers Club и Telegram-чат iOS Good Talks.
- Официальный сайт Peer Lab и Telegram чат Peer Lab Moscow.
- Про Goal-Question-Metric
Автор: Avito
