Еще одна статья о code review

    Что такое code review


    Code review - инженерная практика в терминах гибкой методологии разработки. Это анализ (инспекция) кода с целью выявить ошибки, недочеты, расхождения в стиле написания кода, в соответствии написанного кода и поставленной задачи.

    К очевидным плюсам этой практики можно отнести:
    • Улучшается качество кода
    • Находятся «глупые» ошибки (опечатки) в реализации
    • Повышается степень совместного владения кодом
    • Код приводится к единому стилю написания
    • Хорошо подходит для обучения «новичков», быстро набирается навык, происходит выравнивание опыта, обмен знаниями.


    Что можно инспектировать


    Для ревью подходит любой код. Однако, review обязательно должно проводиться для критических мест в приложении (например: механизмы аутентификации, авторизации, передачи и обработки важной информации — обработка денежных транзакций и пр.).
    Также для review подходят и юнит тесты, так как юнит тесты — это тот же самый код, который подвержен ошибкам, его нужно инспектировать также тщательно как и весь остальной код, потому что, неправильный тест может стоить очень дорого.

    Как проводить review


    Вообще, ревью кода должен проводиться в совокупности с другими гибкими инженерными практиками: парное программирование, TDD, CI. В этом случае достигается максимальная эффективность ревью. Если используется гибкая методология разработки, то этап code review можно внести в Definition of Done фичи.

    Из чего состоит review


    • Сначала design review — анализ будущего дизайна (архитектуры).Данный этап очень важен, так как без него ревью кода будет менее полезным или вообще бесполезным (если программист написал код, но этот код полностью неверен — не решает поставленную задачу, не удовлетворяет требованиям по памяти, времени). Пример: программисту поставили задачу написать алгоритм сортировки массива. Программист реализовал алгоритм bogo-sort, причем с точки зрения качества кода — не придраться (стиль написания, проверка на ошибки), но этот алгоритм совершенно не подходит по времени работы. Поэтому ревью в данном случае бесполезно (конечно — это утрированный пример, но я думаю, суть ясна), здесь необходимо полностью переписывать алгоритм.
    • Собственно, сам code review — анализ написанного кода. На данном этапе автору кода отправляются замечания, пожелания по написанному коду.


    Также очень важно определиться, за кем будет последнее слово в принятии финального  решения в случае возникновения спора. Обычно, приоритет отдается тому кто будет реализовывать код (как в scrum при проведении planning poker), либо специальному человеку, который отвечает за этот код (как в google — code owner).

    Как проводить design review


    Design review можно проводить за столом, в кругу коллег, у маркерной доски, в корпоративной wiki. На design review тот, кто будет писать код, расскажет о выбранной стратегии (примерный алгоритм, требуемые инструменты, библиотеки) решения поставленной задачи. Вся прелесть этого этапа заключается в том, что ошибка проектирования будет стоить 1-2 часа времени (и будет устранена сразу на review).

    Как проводить code review


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

    Pre-commit review


    Данный вид review проводится перед внесением изменений в VCS. Этот подход позволяет содержать в репозитории только проверенный код. В microsoft используется этот подход: всем участникам review рассылаются патчи с изменениями. После того как собран и обработан фидбэк, процесс повторяется до тех пор пока все ревьюверы не согласятся с изменениями.

    Post-commit review


    Данный вид review проводится после внесения изменений в VCS. При этом можно коммитить как в основную ветвь, так и во временную ветку (а в основную ветку вливать уже проверенные изменения).

    Тематические review


    Можно также проводить тематические code review — их можно использовать как переходный этап на пути к полноценному code review. Их можно проводить для критического участка кода, либо при поиске ошибок. Самое главное — это определить цель данного review, при этом цель должна быть обозримой и четкой:

    • "Давайте поищем ошибки в этом модуле" — не подходит в качестве цели, так как она необозрима.
    • "Анализ алгоритма на соответствие спецификации RFC 1149" — уже лучше.

    Основное отличие тематических review от полноценного code review — это их узкая специализация. Если в code review мы смотрим на стиль кода, соответствие реализации и постановки задачи, поиск опасного кода, то в тематическом review мы смотрим обычно только один аспект (чаще всего — анализ алгоритма на соответствие ТЗ, обработка ошибок).
    Преимущество такого подхода заключается в том, что команда постепенно привыкает к практике review (его можно использовать нерегулярно, по требованию). Получается некий аналог мозгового штурма. Мы использовали такой подход при поиске логических ошибок в нашем ПО: смотрели «старый» код, который был написан за несколько месяцев до review (это можно отнести тоже к отличиям от обычного review — где обычно смотрят свежий код).

    Результаты review


    Самое главное при проведении review — это использование полученного результата. В результате review могут появиться следующие артефакты:
    • Описание способа решения задачи (design review)
    • UML диаграммы (design review)
    • Комментарии к стилю кода (code review)
    • Более правильный вариант (быстрый, легкочитаемый) реализации (design review, code review)
    • Указание на ошибки в коде (забытое условие в switch, и т.д.) (code review)
    • Юнит тесты (design review, code review)

    При этом очень важно, чтобы все результаты не пропали, и были внесены в VCS, wiki. Этому могут препятствовать:
    • Сроки проекта.
    • Лень, забывчивость разработчиков
    • Отсутствие удобного механизма внесения изменений review, а также контроль внесения этих изменений.

    Для преодоления этих проблем частично может помочь:
    • pre-commit hook в VCS
    • Создание ветви в VCS, из которой изменения вливаются в основную ветвь только после review
    • Запрет сборки дистрибутива на CI сервере без проведения review. Например, при сборке дистрибутива проверять специальные свойства (svn:properties), либо специальный файл с результатами review. И отказывать в сборке дистрибутива, если не все ревьюверы одобрили (approve) код.
    • Использование методологии в разработке (в которой code review является неотъемлемой частью).



    Сложности при проведении review (субъективное мнение)


    Основная сложность, с которой мы столкнулись, когда внедряли review в первый раз: это невозможность контроля изменений по результатам review. Отчасти это связано с тем, что данная практика применялась без других практик — CI (это еще раз доказывает тот факт, что все инженерные практики должны применяться вместе).


    Утилиты для review


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

    Ссылки






    Пожелания, дополнения, критика приветствуется
    Поделиться публикацией
    Похожие публикации
    AdBlock похитил этот баннер, но баннеры не зубы — отрастут

    Подробнее
    Реклама
    Комментарии 22
    • +5
      А могли бы вы привести статистику о том сколько времени тратится у вас в компании на ревью и субъективную оценку эффективности, т.е. как хорошо оно окупается?
      • +2
        Первый раз начали заниматься review мы где-то чуть больше года назад. Сначала смотрели изменения с помощью обычного diff в Tortoise SVN, затем поставили crucible + fisheye для проведения review. Но этот инструмент оказался неэффективным (это не значит, что он плохой), потому что мы не могли применить результаты review. Отчасти это было связано с тем, что исправления по результатам review не делались, либо их вносить было поздно (продукт уже проходил финальное тестирование и вносить изменения в код уже было нельзя). Так мы мучились в течении 2-3 месяцев, потом забросили это дело.

        Недавно мы вернулись к практике review и как раз начали с тематических review: проводили анализ существующего кода, изменяли его, писали на него автотесты. Тематические review здорово помогают, мы находили большое количество ошибок (мы делали его сидя вместе за одним компьютером), также появилось небольшое понимание, как это должно работать.

        Сейчас мы решили внедрить code review как необходимый элемент итерации в нашем Scrum, и также прикрутить к CI серверу проверку на пройденный review перед сборкой дистрибутива. Так что я думаю, что у нас должно получиться во второй раз.

        С точки зрения эффективности — это безусловно эффективно, даже с учетом нашего небольшого опыта в review.

      • +1
        В последние лет пять считаю очень удобной нотификацию о коммитах по электронной почти.

        Для SVN использовали search.cpan.org/dist/SVN-Notify/lib/SVN/Notify/HTML/ColorDiff.pm

        Для Git использовали github.com/bitboxer/git-commit-notifier (недавно начали использовать доработанные нотификации Gitorious, но мне они не нравятся).
        • –2
          Статья не плохая. Ещё о code review я бы порекомендовал почитать в «Code complete» (Глава 21).
          P.S. А что значит VCS? Может CVS?
          • +9
            Version Control System, вроде бы общеупотребительный термин
          • +2
            Можно проводить code review разными способами — дистанционно, когда каждый разработчик сидит за своим рабочим местом, и совместно — сидя перед монитором одного из коллег, либо в специально выделенным для этого месте, например meeting room.


            В этом вся статья. Не густо :(.
            • +4
              Особенно полезным получился раздел «Утилиты для review» %)
              • +1
                Ссылки неплохие, но там тоже общие слова вида «оно есть, потому что если оно не будет есть — оно сдохнет от голода». Я уже года три с code review сношаюсь работаю, и могу точно сказать — штука чуток посложнее, чем «есть такая фигня, ей еще в микрософте пользуются иногда» :).
              • +1
                Эта статья безусловно не претендует на полноценное описание всей практики, скорее это чисто субъективное мнение на основе небольшого опыта, упорядочивание собственных знаний.

                Можете порекомендовать литературу по code review? Я могу расширить статью дополнительной информацией.
                • 0
                  Слишком молодая дисциплина, хорошая литература отсутствует как класс. Теоретически, немного информации можно подчерпнуть из инструкций и форумов к Klin, Code Collaborator, ReviewBoard. Но у них пока тоже продвижение на ощупь.
              • +1
                Мы для ревью кода используем платный Code Collaborator. Он поддерживает большинство распространённых систем контроля версий.
                Весьма довольны :)
                • –1
                  В продолжение темы предлагаю мою заметку, что такое "статический анализ кода". Он рассматривается как автоматизированный процесс обзора кода.
                  • 0
                    Мы пробовали использовать статический анализатор кода pvs, но к сожалению msvc — не основная среда разработки (пишем время от времени), и поэтому как-то не прижался у нас этот инструмент, возможно с другими анализаторами больше повезет. Кстати, возможно прикрутить pvs studio к CI серверу?
                    • +1
                      PVS-Studio можно использовать из командной строки без .sln файла (http://www.viva64.com/ru/d/0007/), а если есть .sln файл, то проверка из командной строки совсем простая (http://www.viva64.com/ru/d/0001/).

                      Пишите, если будут вопросы по интеграции.
                  • +15
                    Сорри за офф, но уж сильно мне запомнилось фото: вот этих двух парней как раз засняли когда они делали code review
                  • +1
                    Как-то через статью неявно проходит мысль, что анализ кода должен производить не тот, кто его писал? Это обязательное условие?
                    • +3
                      В этом и есть вся суть code review, потому что автор кода относится к свому коду предвзято (грубо говоря, он его считает правильным, идеальным), поэтому задача коллег — сделать этот код еще лучше: указать на опечатки, ошибки, подсказать более правильный путь решения.
                      • 0
                        Когда мы совсем зеленые были, для нас Code Review проводили полуанонимно — не говорили, кто автор анализируемого кода. Все на равных обсуждали. Автора можно было определить лишь по красному от стыда лицу.
                    • +1
                      Используем мердж реквесты в Gitorious для ревью кода. Гибкий и удобный инструмент. Рекомендую.

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