Обеспечение полного просмотра изменений в коде

Бывают ситуации, когда просмотр кода (code review - здесь и дальше буду использовать русский перевод, как бы непривычен он не был) становится обязательным  шагом процесса разработки. Например, в случаях работы с высоконадежной и/или чувствительной частью системы, когда уже недостаточно простого QA-процесса. Open-source проекты живут благодаря просмотру кода. Для них он позволяет масштабировать знания ключевых разработчиков на головы многих участников проекта.

Я бы не хотел затрагивать чувствительные и холиворные вопросы на тему "нужно / не нужно" и "какова цена приносимой пользы?". Поэтому просто расскажу как просмотр кода у нас заработал.

Дано:

• продукт с достаточно коротким циклом выхода новой версии (от 1 недели до 3 месяцев);
• устоявшийся процесс внесения изменений в продукт и выпуска новой версии;
• пост-травматический синдром в половине команды после прошлой неудачной попытки внедрить просмотр кода.

Задача:

Обеспечить 100% (полный) просмотр кода  другими разработчиками (не-авторами) всех изменений перед их выпуском.

Решение (?):

Сразу выбросил топорную версию - поставить обязательный просмотр кода после кодирования и перед QA. Очень уж много недостатков:

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

Некоторое время назад пользовались средствами для просмотра кода - Code Collaborator и Cruicible. При всех удобствах, ими предоставляемых (комментарии, баги, подсветка синтаксиса, история изменений), все же к ним было необходимо было привыкнуть, что часть команды сделать так и не смогла. Кроме того, такие инструменты по факту создавали разработчикам еще один (параллельный) список задач, который был отдельным от системы отслеживания задач (а она очень широко используется), что не добавляло порядка. Иными словами, за задачами на проект и багами разработчик должен был смотреть в таск-трекер, а для просмотра кода - в Collaborator или Cruicible. Да, Cruicible тесно интегрирован с JIRA, но создать общий dashboard или Agile desk с задачами на просмотр кода нельзя.

Есть и еще один важный недостаток - разработчик-автор должен руками создавать задачу на просмотр кода (т.е. включать туда список изменений и назначать ревьювера) - что оставляет уйму возможностей для ошибок вида: забыл, сделал позже, не того ревьювера указал, не те изменения включил, последнее исправление забыл включить и т.п. Причем, чем меньше изменение, тем больше цена дополнительной работы связанной с поднятием задачи. Как следствие, чтобы поддерживать это в работоспособном состоянии требуется  административное давление.

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

Соответственно и ревьювер тоже должен только получать уведомление о том, что есть код, на который ему стоит взглянуть.

Для выполнения цели, нужно следить за количеством кода в версии, который еще не просмотрен и доводить это количество до нуля на предрелизной подготовке, не позже. Отслеживание такой тенденции - дело самой команды, хотя можно поручить это QA или руководителю проекта.

На этом этапе сформировалось понимание наших внутренних ограничений на решение задачи:

1. Просмотр кода не должен тормозить цикл разработка -> QA -> разработка;
2. Минимальное количество мануальных действий от автора и ревьювера, чтобы сделать просмотр кода;
3. Метрика, способная в реальном времени показать количество непросмотренного кода;
4. Задачи на просмотр кода должны быть интегрированы с таск-трекер и (в идеале) не должны отличатся от других задач - на них можно списывать время, заводить подзадачи, переназначать их на других исполнителей и т.п.;
5. Ревьювер должен легко и просто видеть изменения с подсветкой синтаксиса и легко находить историю изменений.

Получилась следующая схема:

1. Разработчик автор пишет и отправляет код в VCS;
2. Некий самописный инструмент (пока не знаем что это) собирает список всех изменений за определенный промежуток времени;
3. Этот же инструмент создает задачи в таск-трекере на просмотр изменений, которые он собрал. Скорее всего, здесь нужна будет какая-то агрегация, потому что коммит = задача на просмотр может породить их слишком большое количество и будет сильно зависеть от стиля работы конкретного автора (кто-то любит частые коммиты, кто-то редкие);
4. Ревьювер получает уведомление о новой задаче на просмотр от таск-трекера. Он также видит её в своем списке задач. Задача должна содержать в себе детали, достаточные для быстрого (в один клик) перехода к коду;
5. Здесь случается уличная разработческая магия: автор и ревьювер обсуждают изменения. Если ревьювер находит ошибки - он заводит их в трекере. Или, что чаще, автор их тут же исправляет. Если выясняется более глобальная недоделка - поднимается дополнительная задача, которая даже может попасть в версию;
6. Ревьювер закрывает задачу (один клик в таск-трекере);
7. Руководитель проекта (Scrum master?) собирает количество открытых задач на просмотр и показывает его в виде графика команде - оно должно выходить в 0 в конце спринта и/или перед выпуском версии.

А где же здесь QA? А там же где и был - сразу (и в параллель) с разработкой. Да, есть риск что QA найдет ошибку которую должен был найти ревьювер - но это должно стимулировать ревьювера быстрее браться за задачи на просмотр.

Осталось прояснить несколько моментов работы "самописного агрегатора":

I. Как агрегировать изменения по времени? Договорились на одном дне (сутках). Так, чтобы утром каждый разработчик мог видеть задачи на ревью кода который вчера написал его коллега. На утреннем стенд-апе он получит устное описание изменений - в задаче увидит код. Удобно.

II. На кого назначать задачи на ревью? Каждый разработчик всегда знает какой-то кусок кода лучше, а какой-то хуже. Этим принципом и стоит руководствоваться при назначении ревью задачи. Каждому разработчику был дан инструмент, в котором он мог отметить в каком модуле системы он хотел бы следить за изменениями. Агрегатор агрегирует изменения по модулям. Если есть несколько ревьюверов в одном модуле - ревью будет назначаться на них поочередно.

III. Что должно быть в описании задачи для более простого ревью? В нашем случае было просто так как наша VCS позволяет создавать URL буквально на все - от коммита до сравнения ревизий файла (а-ля-diff). В описание задачи входит список измененных файлов. Ревьювер в один клик может посмотреть, что было изменено, как про каждый файл, так и про весь пакет. Там же есть ссылки на задачи, для которых эти изменения и вносились. Да, здесь могут попадаться незавершенные задачи, но это ведь не повод ломать сборку и функционал, верно?

Пример:

В системе 3 модуля: GUI, Server и Database.

Есть команда из 3 разработчиков (в скобках модули которые они просматривают): Ivan (GUI, Server), Petr (Database), Anna (Server, Database).

За один день они внесли следующие изменения:

Ivan: Server change 1, Database change 11

Petr: GUI change 2, Server change 2

Anna: GUI change 3

На следующий день они получат следующие задачи на просмотр:

Ivan = GUI change 2, GUI change 3

Petr = Database change 11

Anna = Server change 1, Server change 2
Как видно, по одной задаче на разработчика. Ведь это всего лишь за один день :)


Вот наверное и все, немного цифр в конце. За 2 месяца работы команда из ~15 разработчиков создала ~300 задач на просмотр кода. Распределение было неравномерным - около четырех ревью-чемпионов просмотрело более 30 задач. Что было ожидаемо. Среднее время закрытия задачи - неделя, что наверно много и должно быть уменьшено.

Получилось достаточно объемно, где-то мог быть непонятным.

P.S.
Спасибо Мишегу за реализацию "самописного инструмента".