Еще я знал, что такое «благословение тимлида» — так было на следующем моем месте работы, уже в российском стартапе, где качество кода контролировал серый кардинал, тайно посещавший мои ветки в репозитории и вносивший туда свои корректировочные коммиты. Иногда он не удалял мои строчки, а комментировал их, оставляя рядом пометки вроде «Кто это писал — никогда больше так не делайте» или «Сделал проще», которые четко указывали мне мое место в корпоративной иерархии и вдобавок вызывали у меня жгучий стыд, как у подростка, забывшего стереть историю браузера на семейном компьютере.
Все это закончилось, как дурной сон, когда я устроился работать в 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)
TimReset
28.05.2015 14:37+3точки над ё
— поставить или убрать? А то их в статье нет :)
А какими инструментами в HH пользуются для code review? Что за VCS? А то статья не полная — для совсем уж начинающих, как заметил комментатор выше.vanowski Автор
28.05.2015 15:25+8В принципе, так и задумано. Вернее, задумано было — написать про code review в HeadHunter, а уж на каком уровне это получилось, не знаю. Надеюсь, все же найдется достаточно людей, которым будет интересно! Мы используем пулл-реквесты на GitHub, о чем в статье, кстати, упомянуто :)
TimReset
28.05.2015 15:39-10Т.е. у вас код на GitHub храниться?
TimReset
29.05.2015 19:18+1Меня заминусовали и оставили без ответа. Наверное, я не ясно высказался.
Я первый раз вижу пример, чтобы на GitHub были коммерческие проекты. А уж тем более HH. Я конечно знал, что у GitHub есть коммерческое применение, а не только open source, но чтобы им пользовались вот прямо буквально рядом с тобой, этого я не ожидал.
В связи с чем вопрос:
Почему выбрали GitHub, а не свой локальный VCS сервер? А то я сколько в разных компаниях видел — все исходники хранят у себя.haaji
02.06.2015 02:25+1Могу в общих чертах рассказать.
Почему выбрали GitHub, а не свой локальный VCS сервер? А то я сколько в разных компаниях видел — все исходники хранят у себя.
Не совсем понял, что значит «хранят у себя». Git — распределенная система, у всех разработчиков есть исходники, а github — просто точка синхронизации. В случае, если гитхаб внезапно отключится — вы всегда можете на любом сервере сделать чекаут, дружно добавить этот сервер в remote-ы и продолжать работать, только без удобняшек.
Ну и собственно удобняшки от github, из-за которых его используют:
1. Удобные пулл-ревесты, комментарии, картинки и т.п.
2. Высокий аптайм, свои серваки почаще лежат.
3. Система прав (группы, r/o, все дела).
4. Markdown из коробки.
5. Удобный viewer (блейм/raw/поиск, все дела).
Как-то так.
EvilBeaver
28.05.2015 16:57Чем пользуетесь для ревью? Мы у себя пробуем Crucible, но можем и перенять чей-то опыт.
vanowski Автор
28.05.2015 17:20+2Веб-интерфейсом GitHub — для каждой ветки там можно создать pull request и комментировать его. Некоторые баг-трекеры, например, JIRA, умеют интегрироваться с GitHub и подтягивать в описание задачи ссылки на код и ревью.
DarkByte
28.05.2015 20:58Хотелось бы узнать, почему у HH нет подтверждения адреса электро-почты? Какой то Вася Пупкин зарегистрировал аккаунт с моим почтовым адресом и мне переодически прилетают сообщения с предложениями вакансий, хотя я сервисом пользоваться не планировал. Конечно, когда мне надоело, я зашёл в профиль пользователя и удалил всю информацию, дабы не получать больше вакансий, но по-моему это не лучший выход.
McElroy
28.05.2015 21:12+1есть привычки, странности и простая вкусовщина
На мой взгляд, такие вещи должны регламентироваться принятыми в команде стандартами кода (Code style)Snipe
29.05.2015 10:59-1На каждый чих код-стайл не оформишь )
А на что можно оформить, то то уже лучше не при ревью, а прекомит-хуками или настройками в ide, например )
questor
Уровень статьи — для начинающих, ничего нового не написано.