Процесс ревью кода структурно порочен. Вот, как его исправить
Классический процесс ревью кода страшен до безобразия и даже некоторого восторга. Разберём, почему это так, и что с этим делать.
Взгляд на ревью кода из DALL·E 2
В программировании мы любим задумываться над тем, как поизящнее написать программу, как выбрать методологию поэлегантнее, как бы удачнее применить паттерн. На этом наши возможности не останавливаются, и мы учим наши программы рефлексировать. То есть анализировать самих себя во время выполнения или же достраиваться во время компиляции. Мы учим нашу инфраструктуру автоматически восстанавливаться после сбоев или голосовать.
Когда же дело доходит до того, чтобы подумать, как работаем мы сами, то наступает ступор. С одной стороны, здоровому разработчику над этим думать не хочется, так как здоровый разработчик любит программировать. С другой стороны, доверять проектирование процесса разработки кому-то стороннему, например, менеджеру, тоже совершенно нежелательно. Что он вообще понимает, этот менеджер?! Он, хоть программировать умеет?!
Можно было бы надеяться, что эта история уже каким-то чудесным образом разрешилась, и работаем мы уже хорошо, но нет. Мы только в начале пути. Нам пора начинать задумываться над тем, как мы работаем.
С этой точки зрения мой любимый процесс, в котором просто поразительное количество проблем, — ревью кода. Это тот случай, когда дела обстоят так плохо, что это даже хорошо. В этой статье мы рассмотрим традиционный процесс ревью с разных сторон и разберём пути устранения его несовершенства.
В русскоязычной литературе ревью иногда называется «обзором кода». Как по мне, это название не совсем удачное, и лучше использовать слово «разбор». Ведь мы не просто код собрались на код посмотреть, а основательно в нём разобраться. В совсем классических источниках вообще используется словосочетание «инспекция кода».
Например, такое употребление можно встретить в книге «Факты и заблуждения профессионального программирования»:
Тщательные инспекции позволяют устранить до 90% ошибок из программного продукта до того, как будет запущен первый эталонный тест.
Не стоит безоговорочно верить этим данным, так как в нашей индустрии нет хорошо выстроенной системы выдвижения и проверки гипотез. Гораздо более совершенная система есть в медицине. Но пускай 90% всё равно будут для нас ориентиром. Из книги неясно, о какого рода инспекциях идёт речь. Их, как мы увидим далее, не одна разновидность.
Откуда вообще пошла эта история с инспекциями кода? Дядюшка Боб Мартин любезно предоставил мне ссылку на то, что можно считать первым документальным свидетельством существования инспекций кода. Майкл Фэган (Michael Fagan) задокументировал информацию о них в своей статье 1976-го года Design and code inspections to reduce errors in program development.
Рекомендация дядюшки Боба Мартина начать подступаться к ревью кода с инспекций Фэгана
В статье описано три вида инспекций:
Инспекция решения (design inspection);
Инспекция кода перед юнит-тестированием;
Инспекция кода после юнит-тестирования.
Процесс инспекции кода из статьи Майкла Фэгана
Предполагается, что после каждого этапа происходит исправление обнаруженных дефектов. Ещё я подозреваю, что в те годы юнит-тестирование было дорогостоящим занятием, и поэтому вокруг этого события расположены целых две инспекции.
Из всего набора инспекций в наши дни широко практикуется только один тип — ревью кода. Почему же мы не инспектируем наши решения?
Некоторые предположения об отсутствии разнообразия в видах ревью
Давайте снова обратимся к книге «Факты и заблуждения профессионального программирования». Вот что сообщает нам Роберт Гласс из 2002-го года:
Так почему же инспекции (называемые также экспертными проверками, хотя борцы за чистоту языка различают эти два термина) не прославляются так же, как меньшие «прорывы», например инструменты CASE и разные методологии? Тому есть четыре причины:
1. На инспекциях зарабатывают не многие ведущие производители.
2. В инспекциях нет ничего нового (и, следовательно, пригодного к продвижению на рынок).
3. …
Крайне забавно, что причину в доминировании лишь одного вида ревью сегодня я вижу в прямо противоположном процитированному. Сегодня у нас есть рынок и представленные на нём инструменты. Это известные площадки для хостинга кода: GitHub, GitLab, Bitbucket. Более того, инструменты для ревью кода даже обычно не продаются отдельно, они сразу идут в комплекте с основным продуктом названных сервисов. С учётом популярности pull request’ов вообще сложно себе представить хостинг без инструмента для ревью. Да и кому же не хочется исправить одним махом 90% дефектов?!
А вот где же CASE, UML и прочие инструменты для моделирования решения ещё до кодирования? Их просто некуда добавить. Если действовать по аналогии с кодом, то где-то должна храниться диаграмма, трансформирующаяся затем в программу. Все изменения этой диаграммы должны проверяться внимательным взглядом через соответствующий инструмент. Известные мне инструменты вроде diagrams.net, LucidChart, Miro, Mural и многие другие под это не заточены.
Какие пороки существуют у ревью кода?
В этой части статьи мы рассмотрим ревью кода с разных сторон. Каждая из них продемонстрирует нам свои проблемы.
Процесс ревью с точки зрения BPMN
BPMN — это нотация и модель бизнес-процессов. То есть такой инструмент, с помощью которого можно описать бизнес-процесс. Среди средств описания присутствуют действия, события, логические операторы, сообщения и другие. Если вы временами описываете алгоритмы с помощью блок-схем, я рекомендую BPMN. Это гораздо более выразительный инструмент.
На нижеследующей диаграмме изображён процесс ревью с лишь одним проверяющим. Даже такой процесс таит в себе множество ужасов, чего уж тут говорить о процессе, когда проверяющих несколько. Для построения диаграммы я использовал специальный инструмент, BPMN Sketch Miner, создающий изображения из текста. Если вы также как и я терпеть не можете средства рисования диаграмм как раз-таки за то, что почти всё время занимает рисования, рекомендую присмотреться. Множество возвращающихся конвертов в левой нижней части — это ошибка отрисовки.
Диаграмма классического процесса ревью кода
Всё начинается с создания PR«а. Следующий шаг для автора кода — оповестить ревьюера. В нём выражается любой из возможных способов оповещения проверяющего, будь то устная фраза или сообщение от бота. Далее начинается ожидание, и это самый интересный для нас этап. Продолжительность такого ожидания неизвестна и зависит от загруженности ревьюера. Затем происходит долгожданный разбор исходного кода. Есть шанс, что проверяющий одобрит код сходу, но может случиться и обратное — появятся несколько комментариев с просьбой исправить ошибки.
К этому моменту автор кода может быть уже давненько погружён в другую задачку. Теперь мы ждём его, и здесь снова продолжительность этого ожидания неизвестна. Возвращение к предыдущему коду потребует восстановления контекста. Затем нужно будет как-то истолковать для себя комментарии и отреагировать на них. Настало время оповестить ревьюера.
Не кажется ли вам, что недавно мы уже были в таком положении? Да, были. Мы только что прошли первую итерацию потенциально бесконечного цикла. Да-да, бесконечного. Всегда есть шанс, что ревью отправится на ещё одну доработку. Или же, что мы потеряем к нему всяческий интерес, и оно зависнет.
Хотим ли мы встраивать в наш процесс разработки потенциально бесконечный цикл? Обычно определённость предпочтительнее.
Процесс ревью с точки зрения видения решений
Иногда в наших командах код проверяют старшие разработчики или даже архитекторы. Им хочется, чтобы кодовая база была консистентной, чтобы использовались одни подходы и избегались другие. У них есть видение. У автора кода имеется своё видение. В большинстве случаев автор даже не знает о том, что именно хотят наиболее уважаемые товарищи. Или же забывает, не стоит отметать и этого. Возможно, именно в этот раз просто не получилось сделать, так как обычно делается. Ограничение использования каких-то подходов не приходит просто так и требует вспомогательной среды, постоянных напоминаний, обучения, а такое встречается редко.
Давайте посмотрим на следующее изображение.
Расхождение в видениях автора кода и ревьюера на код с течением времени в классическом процессе ревью кода
На графике видно различие в восприятии автора и ревьюера. После первой итерации ревью начинается схождение, но нам ещё идти и идти.
Представьте себе, что вы наняли бригаду строителей, попросили их построить дом и больше никак с ними не общались до завершения строительства. Как думаете, насколько результат будет совпадать с вашим ожиданием? С большой долей вероятности расхождение будет значительным. Вот и здесь случается что-то похожее. Ведь если у разработчика нет определённого фреймворка, в рамках которого он принимает решения, то они могут быть совершенно неожиданными.
Процесс ревью с точки зрения межличностного взаимодействия
Мем по теме
Картинка говорит сама за себя. Попросите ли вы своего коллегу полностью переделать разработанное, если он он потратил на него много часов работы? Представьте себе конец спринта, задачку, которая вызывала трение на стендапе. Скорее всего, вы поможете своему товарищу влить задачку как можно скорее. С другой стороны, бывают и такие ситуации, когда опытный разработчик проверяет код новичка. В таком случае вариант «начинаем всё заново» вполне может случиться. Но своевременен ли такой вариант? Ведь автор кода свернул на кривую дорожку уже достаточно давно и уже долго продвигался по ней, принимая одно решение за другим.
Процесс ревью с точки зрения бережливой разработки
Бережливое производство ещё многому может научить нашу индустрию. Хотя, стоит отметить, что у нас уже есть «Бережливая разработка программного обеспечения» авторства Мэри Поппендик и Toма Поппендика. Она не особенно популярна, но для нашего анализа будет полезна. В этой книге описываются семь видов потерь, имеющих место в разработке ПО. Это не исчерпывающий список всех возможных несчастий. Просто в бережливом производстве есть традиция изобретать для разных сфер деятельности свои семь типов потерь. Оригинальные потери для Toyota изобрёл в своё время Тайити Оно, отец-основатель производственной системы «Тойоты». Именно эта производственная система вне Toyota стала Lean, то есть бережливым производством.
Семь типов потерь для индустрии разработки ПО следующие:
Частично сделанная работа,
Избыточные функции,
Повторное обучение,
Передачи,
Задержки,
Переключения между задачами,
Дефекты.
Классический процесс ревью кода собрал 6 типов потерь из 7-ми!
