Comments 9
Для начала следует разобраться, что такое код-ревью в команде, и для чего он проводится. Худший вариант - это проводить код ревью "потому что иначе кнопка merge не активна".
Первое что надо запомнить (и донести до разработчиков) - код ревью НЕ ЯВЛЯЕТСЯ средством обеспечения качества программного продукта. За качество продукта (отсутствие ошибок) отвечает сам разработчик кода и принятая на продукте СМК (система менеджмента качества). При определенном везении, вам могут указать на ошибку в коде на ревью, однако сам факт того что на ревью вышел код с ошибкой - уже является "отклонением" в терминах СМК. Попытка сделать из код-ревью инструмент QA приводит к размытию ответственности, и - у семи нянек дитя без глаза...
Код-ревью может использоваться для обучения (например, задача дается джуну или в общем случае человеку который раньше не имел дела с этой частю кода). В этом случае назначается наставник или бадди, который проводит код-ревью ДО предъявления команде (драфты MR тоже можно ревьювить!).
Код-ревью может использоваться в большой распределенной команде как способ всем хотя бы примерно знать что коллеги творят в других частях системы. Чтобы эти код-ревью были эффективны - надо аккуратно писать название и аннотацию к MR. Потому что весь остальной код человек будет читать по-диагонали.
Код-ревью может использоваться для запроса экспертного мнения. Для этого не стесняемся сами ставить комментарий-вопрос в нужном куске и звать туда через тэг нужного эксперта.
Код-ревью может использоваться для того, чтобы был code-ownership и не было скрытых конфликтов в команде. Тогда натурально "Approve" означает: "Да, я подтверждаю что если на ночном дежурстве меня поднимут по проблеме в этом коде - я смогу с ним разобраться и не буду орать 'какой дурак это накодил!". Собственно, момент код-ревью - это последний момент когда вы можете или выразить свое несогласие - или уже молчать вечно...
Наконец, код-ревью может использоваться ретроспективно, вне связи с изменениями, например для анализа кодовой базы на наличие уязвимостей, тех.долга и так далее. Результатом этого обычно являются таски на рефактор в бэклог.
В любом случае, надо избегать ситуаций когда принципиальные замечания появляются на стадии код-ревью. Обычно, это стадия - где слишком поздно все выбросить и переписать (если это только не учебная задача и на нее не заложены ресурсы с многократным избытком - см выше про задачу обучения джунов). Ключевые элементы реализации функционала должны быть проговорены и согласованы на этапе груминга бэклога или крайний срок - на дейлике. Код-ревью - это этап верификации выполнения договоренностей в команде, а не этап нормирования этих договоренностей!
В сухом итоге - код ревью это не сколько технический, сколько социальный инструмент взаимодействия команды. Если это осознать, и не относиться к код-ревью как к магическому инструменту который разом исправит пороки команды - то он сразу начнет лучше работать...
Спасибо и автору статьи и вам за особое мнение. Статья получилась довольно систематизированной. Комент - заставляет задуматься еще больше.
Спасибо за столь подробный и развернутый комментарий!
Я согласен с тем, что ревью не может быть инструментом обеспечения качества, но он вполне может являться инструментом его повышения. Моя логика строится на аналогии с воронкой конверсии в маркетинге, только если маркетинг стремится к ее расширению, я стремлюсь к ее сужению в нижней части - минимизации багов на продакшене. С этой точки зрения и разработка, и ревью, и QA могут (и должны) являться точками выявления проблем.
Сам термин СМК для меня новый, и быстро освоить его в достаточной мере мне не удалось - спасибо за подсвет, постараюсь изучить эту тему. Скажу лишь, что ни разу не наблюдал введенную и работающую СМК (может быть это ошибка выжившего, а может и не самая распространенная практика - не берусь судить). Потому организуем процесс как видим и как можем.
Предложения в статье относятся к ревью, целью которого является повышение качества выпускаемого кода. В вашем комментарии представлены другие цели, которые можно преследовать этим процессом. Далеко не все советы из статьи применимы, если цель ревью иная (например, одна из озвученных). И варианты целей, и способы улучшения процесса для их достижения - темы интересные и не раскрытые. Буду рад почитать об этом)
Живые системы менеджмента качества можно посмотреть в пищевой промышленности на нормальном предприятии. Особенность сегодняшнего пищепрома - это что вы можете не очень напрягаясь отправить на больничную койку от нескольких сотен до тысяч людей (и от десятков до сотен - прямо на кладбище) - привет салатикам от кухни на районе... Соответственно, чтобы такое не происходило постоянно - почти вся пищевка сертифицирована по ISO 9xxx и подобным стандартам.
С точки зрения нас как девелоперов - СМК это тесты, тесты и еще раз тесты. С точки зрения бизнеса - это стратегия в области качества, корпоративная культура, выстраивание отношений с клиентами под определенный стандарт качества и так далее.
Со влиянием код-ревью на качество выпускаемого ПО я абсолютно согласен. Но в голове руководителей зачастую это представляется неправильно. Считается что там где одна пара глаз сделает ошибку, другая пара глаз ее найдет - и вероятность появления корректного кода в поставке возрастет с (1-p) до (1-p^2), где p - вероятность индивидуального разработчика сделать ошибку. Опыт показывает, что такое никогда не происходит. Если расчет идет на это - тогда надо вводить парное программирование - тогда действительно будет 2 пары глаз, и это будет в два раза дороже. Бизнес любит тешить себя иллюзиями, что разработчики, потратив 10% времени на ревью, дадут те же результаты что и парная разработка. К сожалению, волшебства не бывает: сколько ты тратишь на обеспечение качества, столько качества на выходе ты и получаешь...
Во всех случаях которые я наблюдал - повышение качества кода являлось следствием повышения уровня инженерной культуры команды. Код-ревью являлся одним из процессов, поддерживающих это повышение и впоследствие препятствующих естественной деградации инженерной культуры вследствие нормальных энтропийных процессов (ротация разработчиков, изменение бизнес-окружения, и т.д.). То есть, я опять повторюсь - это больше социальный инструмент, чем технический. Нельзя приравнивать код-ревью к автоматическому линтеру в пайплайне. И механизм действия, и задачи у этих двух инструментов разные...
спасибо за хороший комментарий
Честно говоря, комментарий содержательнее, чем статья 🙂 Правда, представить себе, чтобы в реальной жизни всё шло именно так, мне сложно – и к статье, и к комментарию относится. Но стремиться нужно, да – хотел бы жить в таком мире.
Мне понравилось, как это на телефоне выглядит. Согласен :)
Обычно ресурсы опытных разрабов, способных провести ревью, стоят значительно дороже тестеров. Поэтому логично ревью проводить после QA, чтобы не тратить время ревьюеров на выявление багов.
Но если делать ревью после QA, то в процессе ревью возникает необходимость корректировок, и нужно снова отдавать задачу тестерам.
Как правильно организовать процесс?
В рамках своей деятельности основной целью ревью я считаю повышение качества кода. Что такое качественный код - вопрос дискуссионный и может отличаться от компании к компании и от человека к человеку. Для себя выделяю несколько критериев: количество созданных дефектов, простота, расширяемость и работа с ресурсами (диск/память/процессор). Первый критерий можно однозначно определить только постфактум, три последующих обеспечиваются и контролируются (не только, но и) через ревью.
Учитывая стремление выпускать из разработки качественные решения, для меня однозначно, что ревью должно происходить до начала работы QA над задачей.
Не отрицаю при этом возможности ситуаций, когда оптимальной будет иная последовательность - если вы с такими сталкивались, буду рад прочитать/услышать
Улучшаем процесс ревью в команде