Код-ревью изначально ошибочен. Вот как это исправить

Код-ревью изначально ошибочен. Вот как это исправить

7 ноября 2022 г.

В наши дни мы должны критически относиться к тому, как мы делаем что-то в программировании. Нам необходимо применять инженерный подход к нашим процессам. Мы, разработчики программного обеспечения, уверенно говорим об абстрактных классах и чистых функциях. С другой стороны, мы убегаем, когда возникает необходимость обсудить «управленческие» вещи.

Мой любимый широко распространенный процесс программирования, который имеет огромное количество недостатков, – это проверка кода. В этой статье мы рассмотрим это с разных точек зрения и предложим улучшения.

Первый значительный факт, который я прочитал об инспекциях программного обеспечения, был в книге "Факты и заблуждения разработки программного обеспечения" Роберта Гласса. Он утверждает следующее:

<цитата>

Тщательные проверки могут устранить до 90 % ошибок в программном продукте еще до того, как будет запущен первый тестовый набор.

Я не мог определить, относились ли эти слова исключительно к обзору кода. Я отношусь к ним как к разным видам проверок, о которых я расскажу позже.

Дядя Боб Мартин помог мне открыть для себя корни наших современных обзоров. Майкл Фэган сформулировал идею инспекций в 1976 году в статье "Проверки дизайна и кода для уменьшения количества ошибок при разработке программ".

Uncle Bob Martin's reply to the question about code review roots

Я обнаружил там три следующих типа проверок:

  • Проверка дизайна,
  • Проверка кода перед модульным тестированием.
  • Проверка кода после модульного тестирования.

A scheme from Michael Fagan's article on design and code inspections

Работа Фагана не предлагает новый подход к кодированию, а документирует уже существующие явления и аргументирует их. Тем не менее, эта статья является самым ранним письменным свидетельством проверок, которые я нашел.

Проверки кода выглядят как наши современные проверки кода. Почему сегодня мы пропускаем другие виды проверок?

Почему сегодня проводятся только проверки кода: некоторые предположения

Популярность проверок кода и почти полное отсутствие других типов проверок сегодня связаны с нашими инструментами. GitHub, BitBucket или GitLab имеют встроенные инструменты проверки кода, и они естественным образом вписываются в Git, GitHub и другие подходы.

Какой инструмент вы используете для дизайнерской деятельности? Дело не в UI/UX. Речь идет о структуре кода, который вы будете создавать. Возможно, вы слышали об инструментах CASE или UML. Я не видел, чтобы они серьезно использовались ни в одной компании, в которой я работал, а я работал в семи.

На HackerNoon поисковый запрос "UML" дает только два релевантных результата. Таким образом, нет места для введения проверки проекта, когда нет реального процесса разработки решения. В команде, которую я возглавляю, мы использовали Miro для дизайна интерфейса. Процесс мог бы быть более приятным: как и в случае с другими инструментами для построения диаграмм, вы вскоре начинаете рисовать, а не решать свои проблемы. Мы можем быть независимыми от наших инструментов. Вот небольшая цитата из книги "Инвестиции без ограничений":

<цитата>

...если мы просто делаем то, что могут делать инструменты, то мы всегда будем ограничены возможностями наших инструментов.

Какие недостатки есть в существующих проверках кода?

Давайте посмотрим на процесс в его классической форме с разных точек зрения. Каждый из них показывает серьезные проблемы.

Взгляд на BPMN

BPMN — это нотация моделирования бизнес-процессов. Он описывает процессы с действиями, событиями, логическими шлюзами, сообщениями и другими средствами. Я даже рекомендую использовать его, если вы хотите разработать алгоритм, так как он более информативен, чем блок-схема. Давайте изобразим процесс проверки кода для одного рецензента с помощью этой нотации и проанализируем его. I've used a text-based tool to generate the upcoming diagram< /а>. На нем есть небольшой сбой со многими возвращаемыми письмами.

The classic code review process diagram

Все начинается с создания PR, и здесь нет ничего примечательного. Следующим шагом является уведомление рецензента, что представляет собой упрощение всех доступных способов сказать: «Эй, мой PR ждет тебя!👋» Здесь важно освобождение от текущей деятельности. Здесь PR ждет неизвестное время. Затем рецензент погружается в задачу и проводит обзор. Есть шанс, что PR сразу сольют. Однако могло произойти и обратное: появилось несколько комментариев, требующих исправления.

Возможно, автор кода уже находится в следующей задаче, и есть еще одно время ожидания неизвестной продолжительности. Также для возврата требуется восстановление контекста, интерпретация комментариев и их исправление.

Следующий шаг — уведомление рецензента.

<цитата>

Разве мы уже не были там? Да, ты прав. Мы только что закончили нашу первую итерацию потенциально бесконечного цикла. Да, бесконечно. Всегда есть шанс, что появятся новые комментарии. Или что один из периодов ожидания будет длиться вечно.

Хотим ли мы, чтобы потенциально бесконечный цикл стал частью наших повседневных операций? Я не уверен, так как более быстрая доставка обычно предпочтительнее.

Перспектива видения решения

Иногда в наших командах есть старшие разработчики или архитекторы, выполняющие проверку. Они хотят иметь согласованную кодовую базу, придерживаться одних методов и шаблонов и избегать других. У них есть видение. У разработчиков тоже есть своя идея. Обычно они не знают о намерениях старших. Это требует последовательного вещания или вспомогательной среды, что случается редко. Давайте посмотрим на следующую картинку.

The divergence in visions of code creator and reviewer over time in classic code review process

Вы видите, как различаются взгляды участников проверки кода. После первой итерации они начинают выравнивание, но впереди еще много работы. Рецензент корректирует свое видение, а автор кода движется в соответствии с интерпретацией предложений.

Можно использовать метафору "представьте, что вы попросили дом, а потом удивитесь! Это не тот дом, который вы ожидали". Или мы можем посмотреть на его ядро. Представьте, что вы попросили человека добиться чего-то. Затем вы возвращаетесь и видите результаты, но удивляетесь набору решений, которые принял успешный человек. Не удивляйтесь, поскольку вам не требовалась особая схема принятия решений.

Межличностная перспектива

Code review meme

Изображение говорит само за себя. Вы бы попросили своего коллегу-инженера решить проблему с дизайном, потратив на это много дней? Представьте, что ваш спринт закончился, а билет стал причиной некоторого напряжения на стендапе. Вы поможете своему коллеге объединить его как можно быстрее. С другой стороны, может быть старший инженер, просматривающий код младшего. Он мог бы попросить его пройти долгий путь, чтобы исправить решение, принятое в начале. Однако не пора ли давать такие советы? Так много решений уже основано на неправильном выборе.

Бережливое производство еще не повлияло на программирование. Однако у нас уже есть инструмент из книги «Бережливая разработка программного обеспечения» Мэри Поппендик и Тома Поппендик. Этот инструмент представляет собой семь отходов разработки программного обеспечения:

* Частично выполненная работа, * Дополнительные возможности, * Переобучение, * Передачи, * Задержки, * Переключение задач, * Дефекты.

Классический обзор кода получает 6 баллов из 7!🎉

  1. Частично выполненная работа. Задание в обзоре чаще всего забрасывается. Обычно я отношусь к этой стадии разработки как к очереди, а не как к той, где происходит действие. Создание отзыва имеет и интересный психологический эффект: «Задание готово, и это уже не моя работа!» Проверка кода — это территория, на которой никто не несет ответственности, поэтому мы часто видим, как она распространяется на более высокие уровни.
  2. Повторное обучение. Вы можете увидеть переобучение на диаграмме BPMN выше. Восстановление контекста — это переобучение.
  3. Передачи. Если быть честным, это не пустая трата здесь. Это основная механика.
  4. Задержки. Дизайн проверки кода содержит два типа задержек, которые мы обсуждали ранее. Что еще страшнее, так это их петля.
  5. Переключение задач. Люди приостанавливают выполнение задач, чтобы уделить внимание отзыву.
  6. Дефекты. Обзор хорош для выявления косметических проблем, но не недостатков дизайна, которые могут нанести наибольший вред. Упомянутое выше отсутствие мотивации просить стипендиатов о существенных изменениях приводит к существенным дефектам проекта.

Мы рассмотрели проблемы код-ревью со многих сторон. Что мы можем сделать, чтобы исправить этот процесс?

Пересмотр кода

В этом разделе мы рассмотрим те же темы, что и в предыдущем, но с точки зрения исправления.

Исправление структуры процесса

The proposed code review process diagram

Здесь у нас есть автор кода, ожидающий завершения обзора, и сеанс парного программирования после обзора рецензента.

В предлагаемом процессе мы можем увидеть несколько существенных изменений по сравнению с предыдущим:

* Больше нет потенциально бесконечного цикла обзора; * Единичное время ожидания неизвестной длительности, с этим тоже справимся; * Нет необходимости восстанавливать контекст или интерпретировать обратную связь для автора кода. Комментарии теперь служат напоминанием для пары.

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

Устранение расхождений в видении

Что мы обсуждаем на обзорах кода? Решения, принятые в процессе разработки. Некоторые из них мы сделали час назад, а другие неделю назад. Наш выбор стоит один над другим и не является независимым.

Насколько приятной вам представляется возможность подвергнуть сомнению одно из первых решений, на котором основываются следующие несколько сотен? Вы можете быть недовольны этой возможностью.

Самое подходящее время для обсуждения дизайнерских решений — это когда они появляются. Нам нужен еще один тип обзора: обзор дизайна. Иногда, когда проблема сложна, полезно уделить некоторое время плану и обсудить его со знающими коллегами.

Есть известная военная пословица:

<цитата>

Ни один план сражения не выдержит контакта с врагом.

Если мы переведем это на язык систем, то получим что-то вроде: «Системе обязательно нужно будет скорректировать свое поведение, когда поступит первая обратная связь». В случае программирования обратной связью будут проблемы, с которыми мы сталкиваемся при попытке внедрить дизайн в нашу систему. Некоторые решения нуждаются в пересмотре. Нам нужно будет изменить их и снова разойтись в наших взглядах с рецензентом.

Адам Торнхилл в своей потрясающей книге "Рентгеновские снимки дизайна программного обеспечения" предложил следующий метод:

<цитата>

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

Я придумал термин для моей команды, чтобы иметь удобный язык: обзор скелета. Я надеюсь, что это поможет отразить идею, лежащую в основе деятельности.

The divergence in visions of code creator and reviewer over time in the proposed code review process

Бережливый взгляд на предлагаемый процесс

Предлагаемый процесс устраняет или значительно устраняет обнаруженные потери.

  1. Частично выполненная работа. Переключение на другую задачу во время ожидания для автора кода не допускается, поэтому нет значимой для пользователя частично выполненной работы. Частично выполненные действия с меньшим приоритетом являются компромиссом.
  2. Повторное обучение. Переобучения не происходит, так как между завершением кодирования и парным программированием проходит мало времени.
  3. Задержки. Мы значительно сократили задержку рецензента кода и устранили задержку автора.
  4. Переключение задач. Больше не разрешено для автора и управляется для рецензента.
  5. Дефекты. Теперь устранение конструктивных дефектов становится дешевле, а самые важные из них, конструктивные дефекты, исправляются.

Дополнительные мысли

Мы рассмотрели подход к проверке кода для одного автора и одного рецензента. Когда появляется больше рецензентов, проблемы множатся. Так что, к сожалению, не все ошибки становятся мелкими, если вы в последнее время добавляете людей, чтобы посмотреть на все решения, которые вы приняли. Вместо этого вы сделаете слияние кода позже.

Две самые сложные проблемы, с которыми я столкнулся, пытаясь внедрить предлагаемый процесс, заключались в следующем:

Разработчики относятся к этапу обзора как к выполненной работе. Руководители приходят в ужас, когда предлагают ввести резервирование в повседневную работу. Хорошо, что они не управляют бригадами пожарных.

Решение первой проблемы – повторение и скорость.

Вторая проблема более сложная. Здесь я хочу процитировать книгу Даниэля Ваканти «Actionable Agile Metrics for Predictability»:

<цитата>

Существует множество стратегий, которые использует FedEx, но наиболее важной из них является то, что в любой момент времени у FedEx есть пустые самолеты в воздухе. Да, я сказал пустые самолеты. Таким образом, если место будет перегружено или если посылки останутся позади из-за того, что регулярно запланированный самолет был заполнен, то пустой самолет перенаправляется (нужно сказать, вовремя) в проблемное место. В любой момент у FedEx есть «запасные части в воздухе»!

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

Довольны ли мы обновлением? Да, выглядит лучше, чем то, что есть сейчас.

Можем ли мы сделать лучше? Да, можем.

Цель состоит в том, чтобы исключить проверку кода в то время, когда мы можем гарантировать, что все необходимое качество уже присутствует в коде. Чтобы достичь этого, нам нужно создать вспомогательную структуру принятия решений, чтобы помочь разработчикам применять то, что считается хорошей практикой, и избегать плохой. Я никогда не слышал о таком фреймворке, но встроенные в IDE линтеры — это шаг к нему.

Спасибо за чтение! Напишите комментарий, если хотите обсудить описанные идеи.


Оригинал
PREVIOUS ARTICLE
NEXT ARTICLE