Пулл реквесты и асинхронное ревью

Как фича гитхаба убила скорость разработки

Вы, наверняка, каждый день на работе пулл реквесты заводите. А вы знаете зачем? Задумывались ли вы: может есть другой способ? Почему вообще делают пулл реквесты?

История Pull Request’ов

Историческая справка: pull request’ы появились как UX фича Github’а в 2008 году как удобная обвзяка над принятым тогда флоу. В те далёкие времена процедура запроса на стягивание (буквальный перевод pull request, на всякий случай) заключалась в том, что ты пушил код в свой форк и писал мейнтейнеру: “Братан, стяни, плз себе мои изменения”. Ментейнер стягивал, делал код ревью и слал тебя нафиг с твоими правками. Ну или всё-таки пушил в основную репу. Вот и всё. Никакого сложного интерфейса. Никаких кнопок “Approve” и “Merge”. И никаких “LGTM”.

Пацаны из Гитхаба помучались с этим, посмотрели, как другие мучаются и говорят: “А давайте сделаем удобно, как будто мы человеков любим и уважаем”. Это было ещё до покупки Гитхаба монополькой, как вы понимаете.

Pull Request’ы становятся удобной обвязкой над привычным тогда флоу работы в опен сорсе: какой-то хрен с горы наколбасил код в ваш проект и просит его подтянуть. Вы с этим разработчиков общаетесь скорее всего первый и последний раз. И кто его знает, что он умудрился в ваш любименький проектик наговнякать. А тут парни из Гитхаба такой классный инструмент для просмотра придумали. Тебе кто-то отправил ПР, у тебя появилось время заняться своим проектом — ты открыл, посмотрел, накомментил ему. Он пошёл исправлять. Потом погряз в конфликтах, потому что к моменту, когда у него появилось время исправить, ты уже пол-проекта переписал. Красота!

Но как часто у вас рандомные чуваки с улицы коммитят в репу на работе? Как часто вы работаете над кодом, потому что у вас выдалась свободная минутка, а не потому что это ваша прямая обязанность? У вас нет SLA, лид таймов, ТТМов, горящих планов? Вы не доверяете своим разработчикам? У вас нет общего контекста? Вы твёпдо уверены, что асинхронное ревью — правильный подход для вас и вы можете обосновать этот выбор? Или вы делаете асинхронное ревью, потому что “здесь так принято”?

Pull Request’ы дали невероятно удобный интерфейс для асинхронного ревью. Настолько удобный, что всех перестали заботить минусы асинхронного ревью — лишь бы с коллегами лишний раз не созваниваться. А между тем с асинхронным ревью много чего не так.

Теория очередей и переключения контекста

Но какая разница, всё ж работает, ведь так? Если не считать, конечно, влияния на ТТМ и всякий лид тайм, кому он вообще интересен, правда ведь? Ну и нельзя не упомянуть, как все любят делать код ревью. Кроме шуток, если вы прям любите делать код ревью — свяжитесь со мной, хочу посмотреть, как вы это делаете.

Давайте взглянем, что асинхронный код ревью делает с вашим лид тайм и ТТМ. Возьмём простейший случай без зависимостей и конфликтов. Каждый раз, когда вы двигаете задачу в ревью она начинает ждать, когда у ревьювера появится время. После того, как у ревьювера появилось время, и он накомментил — задача снова ждёт! Теперь уже обратно разработчика, чтобы он отреагировал на комментарии. И таких раундов может быть не один.

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

Pull Request’ы и асинхронное код ревью порождают очереди. Очереди убивают TTM и поток задач. Там постоянно все подвисает и чего-то ждет. Когда задача висит в колонке ревью, большую часть времени над ней фактически никто не работает, потому что она ждёт. Здесь привет передаёт уже теория очередей.

Но что же теперь, от код-ревью отказаться что ли? Действительно, с чего мы вообще взяли, что код ревью — необходимая и полезная практика?

Полезно, бесполезно, вредно?

Снова залезем в глубины истории, но на этот раз реально во времена динозавров — 70е. Да, практике “инспекции кода” (запах нафталина и пыли из НИИ прилагается к термину) действительно так много лет. Но и тогда задавались вопросом: “Зачем мы вообще это делаем?”.

В 1976 году Майкл Фэган подошёл к вопросу со всей серьёзностью и выснил-таки, что инспекции кода полезны. Но процесс, который описывает Фэган далёк от привычного нам код-ревью. Там участвовало 3-4 человека: модератор (собственно, ревьювер), дизайнер (отвечал за дизайн системы), кодер (отвечал за код) и тестер (за тест кейсы и их исполнение). Дизайнер и кодер мог быть один и тот же человек. И все они собирались вместе на встрече и изучали код. То есть Фэган говорит о синхронном ревью (не более 2х часов за сессию, иначе долбанётесь). Кроме того, эти инспекции влюкчали в себя так же и ревью дизайна и тест-кейсов, а не только кода — всё, что касалось разработанного модуля системы.

Ещё одна книга из до-пулл-реквестной поры, “Peer reviews in software: a practical guide” Карла Вигерса, так же предлагает синхронный процесс, когда люди собираются на встречу и обсуждают код. Эта книга содержит конкретные рекомендации, как это делать, потому что на синхронном ревью бывали эксцессы. Так, например, один из рецензентов книги Вигерса, Чарльз Эшбахер, пишет: “Мой опыт работы разработчиком заставляет меня очень скептически относиться к экономической эффективности формальных обзоров. На предыдущем месте работы одна из наших первых встреч по обзору настолько быстро вышла из-под контроля, что через пятнадцать минут трое из участников расплакались. В конце концов нам удалось всё уладить, но после этого формальные обзоры проводились в «бархатных перчатках», а обиды не утихали ещё несколько недель. Это привело к затратам, которые почти наверняка превысили все преимущества обзоров. Тем не менее, несмотря на этот скептицизм, я верю в подход Вигерса, если его придерживаться”.

То есть синхронное ревью хоть и полезно, но важны подходы. Бывало ведь такое, что вы смотрели в чужой и орали в монитор: “Ну что за прекрасный человек это написал?” или “Ваш код — удобрение” или даже “Так программируют только Артёмки Татьянычи”? А теперь представьте, что вам это в лицо на встрече несдержанный коллега выдал.

Наша эра

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

Но как мы помним, в 2008 всё изменилось и Github выкатил свои Pull Request’ы для опен сорса. В опен сорсе, замечу, асинхронное ревью было стандартом и до этого. Но в корпоративную среду, судя по всему, оно влезло именно с появлением пулл реквестов.

И вот спустя некоторое время Альберто Бачелли и Кристиан Бёрд из Microsoft решили глянуть, как там дела с код-ревью. И обнаружили, что стало хуже! Так, участники исследования жаловались, что ревьюверы нередко фокусируются на всякой фигне, вроде форматирования, хотя в коде были очевидные проблемы с безопасностью и моделью данных.

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

Можно попытаться сократить размер PR’ов, как того требует Google в своих “бест практисах”. Это должно уменьшить количество требуемого для ревью контекста. Но точно ли практика, требующая таких костылей — лучшая практика?

Стандарт индустрии

Pull Request’ы и асинхронное ревью де-факто стандарт индустрии. Но судя по исследованиям и костылям, которые вставляют, чтобы заставить асинхронное ревью работать как надо, это тот самый пресловутый карго-культ. Наша индустрия вообще любит ждать железных птиц, судя по судьбе гибких методологий разработки.

Pull Request’ы были облегчённым вариантом принятого в open-source cреде процесса “запроса на стягивание”. Можно сказать, что они были прорывом для open source коммьюнити. Но то, что работало хорошо в том контексте, вовсе необязательно будет хорошо работать в корпоративной разработке.

Синхронное ревью показывало хороший результат, но требовало специальных практик, чтобы люди не токсичили. Но и асинхронное ревью страдает тем же самым — одно время на каждой конфе был доклад про то, что нельзя токсичить в комментариях. При этом асинхронное ревью замедляет выполнение задач и заставляет постоянно переключать контекст. За кажущейся свободой сделать ревью “в любое время” скрывается невозможность сделать ревью как следует вовсе.

Когда я впервые узнал про парное программирование, я прочитал, что оно появилось, как выкручивание на максимум хорошей практики код ревью. Я пришёл в индустрию в 2013, а с адекватным код ревью познакомился аж в 2015, но никак не мог взять в толк, как от асинхронного ревью можно было дойти до парного программирования. Но вот ответ: асинхронное ревью в корпоративной среде — новодел.

За счёт появления киллер-фичи Github’а, вся индустрия выкрутила хорошую практику не в ту сторону. Мы решили попытаться сгладить проблемы синхронного ревью за счёт дистанцирования. Мы решили, что так можно ещё и распараллелить работу над несколькими задачами. Мы решили замаскировать проблемы общения переносом его в текст.

Но давайте откровенно — если бы Github не выкатил в 2008 пулл реквесты, делали бы мы сейчас асинхронное код ревью?