До того, как я пришел в HeadHunter, я не знал, что такое code review. Я знал, что такое code approval — так было в одной американской компании, где я начинал свою карьеру, и где весь код в проекте проходил перед мудрыми глазами профессора Фортрана за столиком в глубине офиса. Он с отеческой улыбкой смотрел на мои первые шаги в разработке и говорил: «Вот тут поправь, пожалуйста, и можешь выпускать».




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

Все это закончилось, как дурной сон, когда я устроился работать в HeadHunter.

— У нас есть код ревью, — сказал мне на собеседовании руководитель фронтенд-группы.

— Вау, круто! — ответил я, помня, что никогда нельзя показывать, что какой-то термин тебе не знаком.

Code review — это практика, позволяющая поддерживать качество кода в проекте при помощи его вычитки другими разработчиками перед выпуском задачи. Это значит, что каждый программист, закончив кодить, говорит коллегам: «Ребята, я тут сделал то-то и то-то — пожалуйста, посмотрите, проверьте, что все в порядке, и я нигде не ошибся». Коллеги пробегают глазами его код, отмечают опечатки, семантические, логические и технологические неточности, предлагают их исправить, предлагают более эффективные варианты решения; разработчик соглашается или не соглашается с ними, они находят компромисс, кто-то один назначается ответственным ревьюером, после чего чистый (насколько это возможно) код отправляется в production.

Ревью — увлекательный и сложный процесс, почти как разработка. В зависимости от качеств задачи (сложность, интересность, потенциал для холиваров и взаимного троллинга, собственно код) в дискуссии могут принимать участие от двух до нескольких десятков разработчиков, а длиться она может от пары минут («Привет, есть задачка...» — «Reviewed») до недель («Re: Re: Re: Re: Переменные в глобальной области вид… [23:40]»).

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



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



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



Найти автора.



Понять его.



У ревью, несомненно, есть и минусы. Основной — увеличение времени разработки. Если вы работаете над прототипом или MVP, и вам нужно сделать «быстро и чтобы работало», то ревью с его соблазнами придраться к мелочам может только все испортить. Именно семантические придирки являются самыми распространенными и одновременно самыми раздражающими. Как вампир, впервые попробовавший человеческую кровь, я моментально вошел во вкус после первого попавшегося мне лишнего пробела. Через неделю я уже был беспощадным граммар-наци, не пропускавшим ни одного случайного переноса — только PEP 8, только 79 символов в строке.



Еще один существенный минус — субъективность. Есть алгоритмы, модели, технологии, а есть привычки, странности и простая вкусовщина: коллега может считать, что твой подход — неверный, неэффективный, некрасивый, и на него не будут действовать никакие аргументы. Здесь очень хорошо помогает правило, согласно которому последнее слово в семантических спорах остается за автором.

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

Если вы хотите внедрить эту практику в своем проекте, то хорошим началом будет знакомство со специально придуманными для этого инструментами (например, Stash или Upsource) — чтобы не изобретать велосипед и в конечном счете не скатиться обратно к подходу «я там у тебя поправил». Также, если вы используете GitHub, можете попробовать создавать pull request для каждой новой ветки: это может быть простое описание задачи в двух словах или мини-документация в формате «проблема — причина — решение» со скриншотами вида «было — стало» (особенно если речь идет о верстке). На первых порах это можно сделать опцией, а потом включить в стандартный workflow, как это сделано у нас: если есть задача, обязательно должен быть соответствующий ей pull request.

А когда в команде станет слишком много перфекционистов и охотников за пробелами, на помощь вам придут утилиты и плагины для IDE, следящие за форматированием и синтаксисом еще на стадии разработки. Главное, как во всем, что касается разработки, — не увлечься и не съехать в overengineering — и тогда, несомненно, у вас появится шанс сделать мир еще немного лучше.

@dude Нет, давай лучше напишем «еще немного ближе к идеалу».
@vanowski Нет, это отстой, так не говорят. Просто — “лучше” и все.
@dude Ладно, кавычки и тире поправь, пожалуйста, и точки над ё —

(Титры.)

Комментарии (17)


  1. questor
    28.05.2015 14:33
    +6

    Уровень статьи — для начинающих, ничего нового не написано.


  1. TimReset
    28.05.2015 14:37
    +3

    точки над ё
    — поставить или убрать? А то их в статье нет :)
    А какими инструментами в HH пользуются для code review? Что за VCS? А то статья не полная — для совсем уж начинающих, как заметил комментатор выше.


    1. Nikita
      28.05.2015 14:50
      +2

      используем git и github.


    1. vanowski Автор
      28.05.2015 15:25
      +8

      В принципе, так и задумано. Вернее, задумано было — написать про code review в HeadHunter, а уж на каком уровне это получилось, не знаю. Надеюсь, все же найдется достаточно людей, которым будет интересно! Мы используем пулл-реквесты на GitHub, о чем в статье, кстати, упомянуто :)


      1. TimReset
        28.05.2015 15:39
        -10

        Т.е. у вас код на GitHub храниться?


        1. justmara
          28.05.2015 18:34
          +7

        1. TimReset
          29.05.2015 19:18
          +1

          Меня заминусовали и оставили без ответа. Наверное, я не ясно высказался.
          Я первый раз вижу пример, чтобы на GitHub были коммерческие проекты. А уж тем более HH. Я конечно знал, что у GitHub есть коммерческое применение, а не только open source, но чтобы им пользовались вот прямо буквально рядом с тобой, этого я не ожидал.
          В связи с чем вопрос:
          Почему выбрали GitHub, а не свой локальный VCS сервер? А то я сколько в разных компаниях видел — все исходники хранят у себя.


          1. haaji
            02.06.2015 02:25
            +1

            Могу в общих чертах рассказать.

            Почему выбрали GitHub, а не свой локальный VCS сервер? А то я сколько в разных компаниях видел — все исходники хранят у себя.


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

            Ну и собственно удобняшки от github, из-за которых его используют:
            1. Удобные пулл-ревесты, комментарии, картинки и т.п.
            2. Высокий аптайм, свои серваки почаще лежат.
            3. Система прав (группы, r/o, все дела).
            4. Markdown из коробки.
            5. Удобный viewer (блейм/raw/поиск, все дела).

            Как-то так.


    1. ivanych
      29.05.2015 00:33
      +5

      Хе-хе, а те, кто по скриншотам не узнаёт Гитхаб — это совсем начинающие, или чуть-чуть продвинутые?


      1. justmara
        29.05.2015 10:43
        +2

        Начинающие. Продвинутым приходится напрячься "а не гитлаб ли это?"


  1. leMar
    28.05.2015 14:53
    +5

    А мне понравильось читать. Живо и чувственно написано.


    1. questor
      28.05.2015 15:03

      Не без этого. Мне кажется, что надо бы как-то огородить Хабр в коротких штанишках (проставлять уровень статьи?), чтобы не сливали карму, как это делалось раньше.


  1. EvilBeaver
    28.05.2015 16:57

    Чем пользуетесь для ревью? Мы у себя пробуем Crucible, но можем и перенять чей-то опыт.


    1. vanowski Автор
      28.05.2015 17:20
      +2

      Веб-интерфейсом GitHub — для каждой ветки там можно создать pull request и комментировать его. Некоторые баг-трекеры, например, JIRA, умеют интегрироваться с GitHub и подтягивать в описание задачи ссылки на код и ревью.


  1. DarkByte
    28.05.2015 20:58

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


  1. McElroy
    28.05.2015 21:12
    +1

    есть привычки, странности и простая вкусовщина

    На мой взгляд, такие вещи должны регламентироваться принятыми в команде стандартами кода (Code style)


    1. Snipe
      29.05.2015 10:59
      -1

      На каждый чих код-стайл не оформишь )
      А на что можно оформить, то то уже лучше не при ревью, а прекомит-хуками или настройками в ide, например )