Внедрение инспекций кода в процесс разработки

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


    Когда мы начинали заниматься процессом внедрения инспекций кода (code review) в своих проектах, то были неприятно удивлены отсутствием толковых материалов по организации этого процесса с нуля. Еще один крайне скудно освещенный аспект — это масштабирование процесса инспекций.

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

    Итак, начнем.

    Зачем это все?


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

    1. Уменьшение количества дефектов, которые находят коллеги из отдела контроля качества ПО и клиенты компании.
    2. Удешевление сопровождения приложения за счет улучшения качества кода.
    3. Обеспечение качества и количества unit-тестов.
    4. Обеспечение совместного владения кодом.
    5. Обеспечение обмена опытом внутри команды.
    6. Совершенствование стиля написания кода. Выявление спорных аспектов стиля и обсуждение их внутри команды.

    Кто участвует в инспекции?


    Давайте уточним несколько терминов, которые будем использовать далее в рамках топика.

    Автор (Author) — разработчик кода.

    Инспектор (Reviewer) — разработчик, ответственный за все изменения, попадающие в какой-то определенный модуль или путь в ветке проекта.

    Наблюдатель (Observer) — разработчик, привлекаемый в качестве эксперта.

    Когда инспектировать?


    Теперь определим место инспекций кода в процессе разработки, время проведения инспекции: до добавления кода в репозиторий (pre-commit) или же после его добавления (post-commit). К выбору стоит подойти осмотрительно, поскольку процесс внедрения инспекций кода часто бывает болезненным. Наибольшему риску подвержены команды, в которых преобладает «личное» владение кодом (а это случается сплошь и рядом). Поэтому разумно сначала внедрить практику инспекций post-commit, чтобы свести к минимуму риск срыва сроков по проекту из-за неизбежных поначалу «холиваров». По мере накопления участниками проектной команды необходимого опыта — можно переходить к инспекции pre-commit.



    Надо сказать, что для себя мы изначально выбрали pre-commit инспекции.

    Как это работает?


    При создании инспекции разработчик добавляет следующих участников:

    1. инспектора из своей группы;
    2. руководителя своей группы.

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

    Руководители групп назначают инспекторов из своих групп.



    Такой подход обеспечивает децентрализованное назначение участников инспекции и отлично масштабируется как по вертикали (в иерархии), так и по горизонтали (при увеличении числа проектных групп).

    Что нужно для внедрения?


    Для успешного внедрения инспекций кода необходимо выполнение нескольких условий.

    • Перед попаданием в репозиторий любой код обязательно просматривается как минимум одним человеком, который хорошо в нем разбирается.
    • Группа разработчиков всегда в курсе изменений, которые вносятся в их проект другими группами.
    • Руководитель группы в курсе всего, что делает группа, и получает достаточное представление о любом коде своей группы.
    • Внутри группы разработчики имеют достаточное представление о коде, который пишут их коллеги.

    При выполнении этих условий достигается хороший уровень общего владения кодом участниками проекта.

    Вот, пожалуй, и все :)

    Если тема инспектирования кода и описание нашего опыта будут интересны хабрасообществу, то одну из следующих статей мы посвятим автоматизации процесса инспектирования с применением инструмента CodeCollaborator компании SmartBear.



    Благодарим за внимание, и до новых встреч!
    Positive Technologies
    245,00
    Компания
    Поделиться публикацией

    Похожие публикации

    Комментарии 18

      0
      Чем дольше код стоит на инспекции, тем больше шансов, что код в репозитории изменится, придется мерджить и вновь выставлять на инспекцию. Такой процесс может затягиваться на недели. Пока у нас нет стандартного решения этой проблемы и каждый подобный случай решаем индивидуально. Возникают ли у вас такие ситуации?
        +1
        Мы для избежания таких проблем стараемся во-первых, ставить задачам по инспекции более высокий приоритет по сравнению с обычной разработкой, а во-вторых разрабатывать небольшими законченными кусками, что бы инспектору не приходилось неделями сидеть и разбираться что же там такое написали.
          +2
          Мы решаем эту проблему так же, как описали здесь:

          1. Мы реализуем новый функционал в отдельных ветках.
          2. Каждая большая задача всегда разбивается на подзадачи.
          3. Разработчик отдаёт код на инспекцию вменяемыми законченными частями.
          4. Приоритет на инспекцию у нас ниже только критических уязвимостей.
          +1
          Все верно. Не представляю как вообще можно мержить чью-то ветку, не просмотрем код и не прогнав тесты.

          У нас в команде принято так:

          До ревью отвечает за код разработчик, после ревью за возможные проблемы отвечает тот, кто просматривал и мержил код. В таком случае ответственность за свои действия на достаточно высоком уровне и народ перестает писать что попало.
            +3
            Я правильно понял, что вы написали, что разработчик перестает нести ответственность за свой код после ревью? Так что тогда ему мешает писать код таким образом, лишь бы он прошел ревью, а если будет глючить потом, то и так сойдет?
              0
              Да, вы поняли верно. Но поскольку ревью делаю его коллеги, тоже такие же разработчики, с которыми он работает непосредственно каждый день, этот способ работает.
                0
                Ну люди бывают разные, и у некоторых экземпляров отношение к работе наплевательское.
                Хотя таких, конечно, лучше сразу увольнять…
            +1
            У нас всё проще — в системе контроля версий стоит pre-commit hook, который проверяет наличие строки «reviewer: %username%» или «pending-reviewer: %username%». При её отсутствии коммит не проходит.

            Соответственно, если изменения кода существенны, ты зовёшь на ревью человека, который хорошо разбирается в коде, с которым ты работал. Если же это какие-нибудь незначительные поправки — вписываешь этого человека как pending-reviewer, а затем с помощью FishEye генерируется post-commit review.
            • НЛО прилетело и опубликовало эту надпись здесь
                0
                Ага. Контроль версий — SVN.
                • НЛО прилетело и опубликовало эту надпись здесь
                    0
                    Есть сборочный сервер (Bamboo), несколько раз в сутки собирает всё и гоняет тесты. А поверхностно убедиться можно и запустив программу/тесты локально.
              +2
              CodeCollaborator — отличная штука. На прошлом проекте купили лицензию и с удовольствием пользовались.

              Справедливости ради стоит отметить, что если ваша система контроля версий поддерживает shelving (как, например, Perforce), то ревью кода можно проводить и просто через электронную почту, эффективность это не слишком убавляет.
                +2
                Мы в команде практикуем pre-commit review: написал код -> сделал патч -> отправил на группу (email) -> получил два одобрения -> commit. По патчу, однако, сложно судить об общей логике изменений. Лучше всего проверяется стиль кодирования :) Но и ошибки находятся, так что практика однозначно полезная.
                  0
                  pre-commit review на практике оказывается уж очень напрягающим
                  Мы просто задачу из in progress перевешиваем в review, и только потом в test/done. review проходят обычно на следующий день, в редких случаях — через неделю.
                    0
                    А насколько это удобно работать через патчи если quilt не используется? Ну то есть — есть таск в трекере, кто то сделал бранч, поработал, получил патч, выложил в трекере, его посмотрел ревьювер, ок, потом запушили. Не очень удобно для не больших проектов.
                    Вот другой случай — идет большая монотонная работа по разработке, дескритизация тасков низкая. Хочется ревьювить ежедневные комиты. Чем удобно пользоваться?
                      0
                      Мне кажется что мы с Колей Алименковым достаточно хорошо раскрыли тему практики Code Review. Не буду много писать — если интересно, посмотрите запись нашего доклада на AgileDays 2011 vimeo.com/22736643
                        0
                        Исходя из заголовочной картинки, Code Review оставит вас без штанов!

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

                        Самое читаемое