В рамках предыдущей статьи "По следам код-ревью" меня попросили написать поподробнее про инспекцию кода в своей команде:
Интереснее как такое внедрить в команде:
- кто проводит
- с какой периодичностью
- как исправляете (задачу создаете или еще как)
- как контролируете выполнение исправления (вдруг разработчик просто забил на замечание)
- как боретесь с вкусовщиной (когда у разных разработчиков разное понимание стандартов)
- всех проверяете или выборочно
и
Но про организационные аспекты было бы не менее интересно узнать.
На каких проектах код-ревью целесообразно применять?
Какие трудности возникают?
Как долго применяете?
Сколько времени тратится и каких специалистов?
Очень полезно было бы узнать ответы на эти вопросы.
и другие.
У меня появилось немного свободного времени, так как троих мелких я развез по бабушкам на текущий месяц, то выполняю свое обещание. Рекомендации базирую на своем опыте, выражаю свое мнение и результаты размышлений. Саму статью я решил сделать в формате вопрос и ответ.
Во-первых, хочу обратить внимание, что для код-ревью вы и ваша команда должны «созреть»! Учтите, убеждать вас внедрять эту методологию с пеной у рта, что это должно быть везде и у всех я не буду.
Во-вторых, этот процесс внедрения является самым настоящим проектом, поэтому у него должен быть руководитель – тот человек, который начнет и завершит его. Само собой, это не поедет.
В-третьих, у каждой команды свой особенный и индивидуальный процесс, поэтому процесс запуска и результат на выходе будут иметь какие-то свои особенности. Возможно какие-то рекомендации или шаги придется дорабатывать под себя.
Если по первым пунктам вы готовы, тогда поехали!
Что такое код-ревью?
Код-ревью (code-review), обзор кода, инспектирование кода - это инженерная практика в терминах гибкой методологии разработки, анализ и проверка кода с целью выявить ошибки, недочеты, расхождения в стиле написания кода, в соответствии написанного кода и поставленной задачи.
Процесс ревью - это процесс контроля того что задача выполнена в полном объеме, соблюдены все правила и договоренности, что решение не избыточно, что его легко поддерживать и развивать в дальнейшем.
Что нам дает код-ревью?
Разработка без code-review и с code-review.
Нам обзор кода привнес следующие бонусы (простыми словами):
- Это мощная проверка того что делает джун или новичок.
Ему становится проще настроиться на одну волну или получить опыт. - Позволяет отсечь ошибки.
Быстро находятся глупые ошибки и опечатки. Хочу заметить, тут нет цели найти все 100% ошибок (это вопрос тестирования), нашли несколько уже здорово. - Повышается ответственность.
Человек, который знает, что его будут проверять, уже пишет лучше. - Повышаем свою квалификацию.
После многократного разбора ошибок и методик "проблемного" кода стало значительно меньше, и соответственно качества кода выше. - Уменьшается эффект bus factor (эффект автобуса).
- Позволяет выбрать наиболее лучшее решение.
Проверяющий (щие) может не согласиться с реализацией и тогда проблема эскалируется и формируется коллегиальное обсуждение. - Снижаются временные затраты на исправления ошибок и необходимости в доработках после релизов, а также уменьшаются репутационные потери.
- Общий код выглядит лучше.
В коде становится меньше "макарон". - Повышается степень совместного владения кода.
Команда мыслит на одной волне. Появляется согласованность в принятии тех или иных решений.
Что требуется для внедрения методики код ревью в процесс разработки?
Ответ:
- Желание или необходимость.
- Система баг-трекинга.
Это то место, где проходит основная коммуникация в рамках процесса разработки. По моему мнению, без этого инструмента говорить про процесс разработки бессмысленно. - Бизнес-процесс разработки.
Описание процесса жизни задачи или по крайней мере понимание что и зачем и кто, и что делает. Статусы и описание назначения и процессов, происходящих в этом статусе. - Команда.
Если в команде 1 человек, то данную методику применить невозможно.
Как внедрить код-ревью?
Ответ:
- Понимаете, что вы этого хотите. Что вы «созрели». Что вы готовы это сделать?
- Добавляете в свой процесс новый статус: «код-ревью» и/или «требуется код-ревью».
- Описываете как задачи попадают в этот статус, что требуется сделать в этом статусе, что потом следует.
- Определяетесь с ответственными по проведению код-ревью.
- Определяетесь с внутренними стандартами. Согласуете определенный стиль и подход.
- Определяетесь как будет происходить коммуникация в рамках проведения код ревью, как что и где смотреть.
- Определяетесь, когда по времени будет проводиться код-ревью.
- Начинаете работать.
По части тезисов будут расшифровки ниже по тексту.
Дорожная карта от желания «хочу внедрить код-ревью» до успешного использования в команде
Кто может выполнять код-ревью?
Ответ:
Для выполнения код-ревью должны быть выбраны наиболее опытные специалисты универсалы или более узкие специалисты в некоторых направлениях. И далее назначения ответственными на задачи в части код-ревью выбираются в зависимости от сложности задачи и ее области изменения.
Далее по достижении некоторого профессионального уровня к процессу подключаются бывшие джуны.
Также для ответственного по код-ревью у нас есть маленькая инструкция шпаргалка, на что обязательно смотреть и что проверять.
Замечания:
Существует мнение код-ревью должен выполнять каждый участник команды выбираемы рандом, но. Вот тут крайне не согласен, вы бы доверили провести код-ревью джуну, когда на кону стоит изменение механизма взаиморасчетов или отправки данных в налоговую?
Либо код-ревью должна проводить вся команда. Не согласен также, конечно здорово если все 10 человек посмотрят каждую задачку, но когда кодить будем? Команда привлекается (или ее большая часть) в моменты кода возникает спор между участниками (эскалация), либо они сами призывают для помощи других участников.
Задачи на код-ревью выдаются на команду и может взять кто-то любой? Это здорово, но всегда будет какая-то сложная задача или сложный автор, которого будут всячески избегать, и еще если задачка не очень важная. Да, еще все будут думать друг на друга: Вася сделает, а Вася будет думать, что пусть Петя возьмет. При таком подходе у нас на ревью задача как-то висела месяц, она была еще и не особо важная (низкий приоритет).
Должен быть назначен один ответственный по код-ревью? А вот дальше можно меняться задачками друг с другом, совещаться, призывать коллективный разум.
Нужен ли кто-то для контроля?
Обязательно должен быть ответственный человек, который все этим управляет и контролирует (по крайней мере на старте). Если не будет ответственного, то это все дело обязательно скатится в хаос и нечего не получится.
Как выглядит процесс разработки с учетом инспекции кода?
Ответ:
Привожу пример процесса с использованием инспекции кода:
Задача принимается в работу, а после отправляется в соответствующий статус, который требует проведения обзора кода (можно провести аналогию с pull-request).
Далее ответственное лицо принимает задачу в работу (статус "Код-ревью") и проверяет. Результатом его работы будет принятие задачи или возврат на доработку (более подробно процесс взаимодействия с примерами см. "Процесс проведения код-ревью?").
На рисунке приведена доска SCRUM (см. ниже "Пример доски задач SCRUM"), которая содержит в себе представление процесса разработки.
В колонке "Код-ревью" находятся (объединены) задачи с двумя статусами "требуется код-ревью" и "код-ревью" - это колонка инспектора по коду).
В колонке "В работе" объединение статусов "В работе" и "Возврат" - это колонка разработчика.
Пример доски задач SCRUM
Процесс проведения код-ревью?
Ответ:
Мы проводим код-ревью в рамках бизнес процесса. Если задача не прошла код ревью, тогда она возвращается в работу. И после завершения опять переводится на код-ревью.
Наличие подтвержденного код-ревью у нас проверяется при формировании сборки.
Результатом код-ревью в задаче должна быть отдельная отметка [КОД-РЕВЬЮ] с информацией о том, что она пройдена.
[КОД-РЕВЬЮ]
ОК.
В случае наличия замечаний, то автору приводится краткий список проблем:
[КОД-РЕВЬЮ]
1. Изменить форматирование
2. Используем запросную модель, а не объектную в модуле объекта….
3. Возможна ошибка при использовании…
Автор при возврате отвечает оппоненту:
1. Сделал
2. Сделал
3. Ошибки нет, т.к.
Как погасить эскалацию конфликта ревьювера и автора?
В процессе выполнения диалога между автором и проверяющим может произойти тупиковая ситуация. Когда автор не согласен с замечаниями инспектора, а инспектор по коду не пропускает решение оппонента. Тогда должна происходить эскалация обсуждения на более высокий уровень и могут привлекаться коллеги или призываться сразу "арбитр".
Последней инстанцией при эскалации споров должен являться самый опытный сотрудник, архитектор (главный судья или арбитр) или что-то вроде комиссии. Оставлять последнее слово за автором не совсем хорошая идея.
Что такое дизайн-ревью (desing-review)?
Можно сказать так - это проверка архитектуры кода предлагаемой разработчиком. Ее рекомендуется выполнять до начала процесса кодинга, после принятия задачи в работу и проведения первичного анализа. Можно собраться группой и выслушать предложения разработчика о выбранном им проектном решении, либо совместно обсудить какое решение лучше принять. Для проведения этого обзора лучше всего привлечь в "комиссию" самых опытных разработчиков и/или архитектора.
Благодаря использованию desing-review вы многократно усиливаете эффект code-review. Тем самым вы устраняете ошибку выбора проектного решения, до момента когда ее устранить довольно сложно или уже не возможно.
Для разработчика вы разбиваете задачу на два этапа:
- "я понимаю, что и как собираюсь делать"
- "я делаю"
Когда вы выполняете этот пункт то необходимо ответить на следующие вопросы:
Можно сделать проще и удобнее?
Нет ли избыточности?
Соответствует ли задаче реализация?
Это костыль или нет?
Используются типовые решения или это новый велосипед?
Соответствует ли паттерну открытости/закрытости (если соответствует, то при доработках не потребуется переписывать все решение снова, а только добавлять новый код)?
Что смотрим при проведение инспекции кода?
Можно сформулировать в общем случае следующий набор вопросов на которые должен ответить инспектор кода:
Соответствует ли код стилю? Соблюдаются ли правила, стандарты и соглашения принятых в команде? (об этом ниже см. "что за стандарты и как их сделать?")
Возникают ли у меня сложности в понимании?
Можно ли что-то исправить путем рефакторинга?
Хорошие наименования переменных, функций?
Есть ли излишнее усложнение кода? Высокая ли цикломатическая сложность (фактически на сколько большая вложенность если иначе)?
Соответствуют ли запросы критериям оптимальности?
Дополнительно еще можно составить некоторый чек-лист критериев проверки, на первых порах выполнять проверять в его рамках.
Что за стандарты и как их сделать?
Ответ:
Под стандартами необходимо понимать определенный набор правил, которые вы должны соблюдать и использовать в процессе разработки. Лучше сделать какой-нибудь документ с основными выдержками и ссылками на документы и стандарты. Вот что мы обсуждали:
1. Вы договариваетесь о правилах форматирования кода.
Порядок оформления комментариев, названия переменных, создания обработок, форматирования текста и т.п.
2. Вы договариваетесь о правилах доработки типовых конфигураций.
В нашем случае мы договорились о правилах минимального изменения конфигураций.
3. Используете правила и рекомендации по работе с БСП.
Нравится вам или нет, но к примеру, добавление нового присоединенного файла к производителям необходимо делать в соответствии с документацией от разработчиков.
4. Используете рекомендации по разработке от 1С.
5. Свои стандарты, правила и т.п.
Пример содержимого документа памятки:
А) Добавляем новые реквизиты и метаданные с префиксом ццц_
Б) Перед началом вставки кода и завершении добавляем специальные теги (используем шаблоны кода)
//++ Номер Задачи Автор
//-- Номер Задачи Автор
В) Форматирование кода используем типовое
Г) Делаем коммиты на каждую задачу отдельно и при помещении в хранилище в комментарий добавляем метку с номером задачи
Д) и т.д. …
Кого проверяем? Всех или выборочно?
Ответ: Каждая задача, в которой было изменение кода или метаданных, должна пройти код-ревью. Соответственно задачи типа прочитать книжку по постижению дзена код-ревью не требуют)
Когда проводится код-ревью и как часто?
Ответ: Мы начинаем проведение ревью каждое утро с 9 часов утра, пока не закончатся задачи.
Либо проверка может выполняться по требованию в процессе рабочего дня (срочный баг фикс и т.п.).
И конечно по желанию.
Как проводится контроль?
Один из этапов контроля - это процесс выполнения сборки. В каждой задаче, которая пришла в хранилище сборки должен быть пройден статус "код-ревью" и должна быть отметка о проведении "код-ревью".
Перевод в статус с код-ревью выполняет только ответственный по "код-ревью".
А по факту есть определенная ответственность - отметка код-ревью должна быть. У нас это "устаканилось". Да, были разговоры и сложности в процессе, сейчас это естественно и проблем не возникают.
Как выгляди диалог общения внутри процесса ревью?
Ответ: Допустим только профессиональный диалог. Никаких личных отношений, переходов на «ты», высмеивания или пренебрежительного отношения.
Запомните цель code-review повысить качество вашего продукта, а не поднять настроение команде.
Правильный и не правильный процесс code-review
Что пишет автор перед отправкой на код-ревью?
Перед началом код-ревью автор решения должен подготовить список изменений, обозначить их в коде. Пример:
а) В задаче автор обязательно отмечает список измененных метаданных (еще несколько отдельных требований).
б) В хранилище конфигурации задачи обязательно помещаются с комментарием о номере задачи или номерах (желательно чтобы коммит был в рамках одной задачи).
в) В коде конфигурации подключенной к хранилищу разработчик отмечает начало изменения и конец изменения. Согласно стандарта (см. выше)
// ++ Задача-6800 Иванов
// комментируются данные до
// Если Объект.УчитыватьНДС Тогда
Если Объект.УчитыватьНДС и Объект.ццц_Учитывать
….
//-- Задача-6800 Иванов
г) Если ведется разработка в EDT, то на код ревью отправляется номер ветки. Скажу честно, то в EDT сравнивать очень непривычно.
Как боретесь с «индивидуальными» (вкусовщиной) подходами?
Ответ: Есть общий стандарт и договоренность внутри команды. Все остальное вне закона.
Иными словами, если приходит задача и она не соответствует стандарту, то задача «сразу» отправляется в возврат.
Что писать в замечаниях к код-ревью?
Все замечания мы пишем в рамках тега «[КОД-РЕВЬЮ]» в нумерованном списке. Соответственно, автор при исправлении и объяснении своей позиции будет отвечать на каждый пункт из пронумерованного списка.
Если нет замечаний, то ставится резюме – «ОК». (пример оформления переписки см. пункт «Процесс проведения код ревью. Как проводится контроль?»)
Подтверждаем сове мнение примерами. В рамках проведения ревью будет правильно привести пример, на то как это было бы сделать правильно – особенно важно для новичка или джуна.
Делаем ссылки на стандарты или правила. Все свои рекомендации, версии должны быть подкреплены доказательствами. Фразы не допустимы: "Я так считаю", "Мне нравится" и т.д.
Соответствие стандартам. Если не соответствует стандартам так и пишите: не соответствует стандартам.
Какие есть автоматизированные инструменты?
Здорово если бы для 1С появился мощный статический анализатор кода типа PVS-studio, но пока его нет.
Мы использовали проверку конфигурации через тестирование и исправление, типовая проверка кода. Еще можно взять автоматизированную проверку конфигурации на стандарт - 1С соответствуют.
Про остальное не знаю. Внедрять и поддерживать полу-функциональные инструменты не считаю эффективным подходом.
Как происходит проверка технически?
В двух словах - используем имеющиеся типовые инструменты от 1С:
- В конфигураторе используем обязательно хранилище и функционал сравнения истории с выборочным объектом. В данном случае удобно использовать метки в коде для выделения изменений по задаче от всего потока изменений.
- В EDT используем функционал сравнения веток (тут мы можем сравнить даже формы). Также для сравнения кода модулей можно использовать функционал сравнения с самого GitHub (но без форм).
- Некоторые команды используют другие инструменты для разбора конфигурации. Это совсем отдельный вопрос.
Сколько времени тратится на код-ревью?
Мы рекомендательно ограничиваем время на проведение процедуры код-ревью 15-20 минут на задачу. Однако на этот временной параметр влияют множество факторов: уровень автора (эксперт или джун), сложность задачи, риски и последствия плохого исполнения задачи (фактор риска), возможная эскалация и призыв коллективного разума и т.д.
Вообще не рекомендуется единоразово тратить на проведения обзора более 60 минут. А также передавать на ревью задачи с большим измененным количеством метаданных и тысячами строк кода (читал что нормально строк 200, а 500 - это максимум, дальше уже начинается треш и угар). Поэтому получается, что оптимальное количество задач на утреннюю инспекцию кода - это 3-5 средненьких задачи.
Как давно применяете?
Уже очень давно, минимум 5-6 лет.
Какие трудности возникают?
Их много особенно в начале пути.
В процессе проведения ревью довольно сложно проверять очень большие задачи по объему, изменения в которых затрагивают большой пласт кода и/или большое количество метаданных. В данном случае изначально рекомендуется разбивать большие задачи на маленькие.
Тяжелее проверять задачи в EDT, не смотря на наличие ветвлений. Тут проблемы технического характера, а также самого IDE. Разработчики от компании 1С обещали поправить, ждем.
Возникает борьба с уложившимися подходами новых разработчиков. Стандарт понимают, но сложившиеся годами навыки дают о себе знать.
Приходится всем следовать стандартам. Иногда так хочется запилить «по-быстрому», но нельзя.
Сам процесс внедрения и улучшение процесса вызывает определенные «сложности». Это, на мой взгляд, логично и понятно. А изменения должны быть, т.к. время течет и все меняется)
Заставить проводить код-ревью утром в понедельник). Поэтому мы ненавязчиво напоминаем коллегам об обязанностях.
Как повысить уровень, коммуникацию, эффективность внутри команды?
Ответ:
Для этого рекомендую проводить краткие вебинары (15-30 минут). Сейчас организовать подобные мероприятия не проблема – есть Skype, MS Teams, WebEx и др.
В первой части проводится ретроспектива и каждый из участников отвечает на три вопроса:
Что было плохого?
Что было хорошего?
Какие есть предложения?
Во второй части проводим обезличенное обсуждение найденных косяков, интересных находок, мастер классы и т.п.
На каких проектах целесообразно применять код-ревью?
Ответ:
Практически на всех. Не забывайте, что даже эксперт может «накосячить».
Для проектов в которых планируются изменения, доработки в будущем, будет передаваться другой команде - инспектирование кода MUST HAVE. Т.е. если проект большой (объемный по времени сопровождения, внедрения и т.п.) и на нем отсутствует код-ревью, то я считаю это преступлением против принципов хорошего тона в разработке).
Излишне, наверное, будет инспектировать код при разработке прототипа и минимально жизнеспособного продукта (обработки, отчета на раз). Хотя и тут если особенно исполнение хромает, то лучше одним глазком посмотреть.
