Представьте себе, что вы работаете над относительно простым монолитным приложением в команде из пяти человек. К сожалению, каждый из вас выгружает код в основную ветку всякий раз, когда завершает работу над порученной ему функцией (или, что еще хуже, когда захочет). Вы то и дело правите код друг друга: используете разные соглашения по именованию, переписываете и, возможно, дублируете код. Именно так может выглядеть разработка в отсутствие процесса код-ревью.
Если вы работали в такой команде, без обзоров кода и проверок качества как части конвейера CI/CD, вы наверняка сталкивались с этими болевыми точками на собственном опыте. Много времени тратится на исправление того, что вообще не следовало развертывать в продакшен. Откат до последней рабочей версии теперь практически невозможен. И что делать с багами, то и дело возникающими в унаследованном коде? Вы постоянно повышаете прогноз времени, которое понадобится для исправления ошибок, ведь код довольно запутанный. И конечно же: разработчик, который все это написал, давно покинул команду.
Чем более многочисленна команда, чем сильнее она распределена, чем сложнее приложение, тем острее ситуация. Представьте себе, что ваше приложение — это двадцать пять микросервисов, пять из которых поручены вашей команде, состоящей из десяти разработчиков, разбросанных по всему миру. Возьмите симптомы из прошлого примера и примените их к этой ситуации. Вы получите очень дорогостоящие ошибки.
Возможно, что эта болевая точка — отсутствие процесса код-ревью — и стала причиной, по которой вы начали читать эту книгу. Может быть, вы уже применяете то, что узнали из первой и второй частей, а значит, одну проблему уже решили. Но к сожалению, есть и другие, которые могут внезапно появиться уже после внедрения проверки кода. Именно о них я и расскажу вам в этой главе.
Болевые точки код-ревью
Обзоры кода будут прекрасно работать и помогать нам лишь при условии, что мы сумеем наладить последовательный, сопровождаемый и эффективный процесс. В противном случае по множеству причин они рискуют превратиться в нечто неприятное и нежелательное. Что препятствует проверкам кода или постепенно сводит на нет их эффективность? Обычно это недостаток общения и доверия между членами команды или отсутствие четких правил. И то и другое приводит к возникновению типичных болевых точек, которые ухудшают проверку кода до такой степени, что проще обойтись без нее. В этой главе мы рассмотрим несколько характерных типов код-ревью, которые точно нам не нужны.
Ленивые код-ревью
Один из типов обзоров кода, который является основным источником разочарования для разработчиков, — ленивые код-ревью, когда по тем или иным причинам проверки выполняются неэффективно, в спешке или ненадлежащим образом, несмотря на наличие вполне сформировавшегося процесса. Ленивые код-ревью бывают нескольких видов.
Синдром «По-моему, неплохо»
Вы получили уведомление по электронной почте: коллега направил вам на рассмотрение свой новый пул-реквест. У вас есть немного времени, и вы решили взглянуть. Изменено 32 файла, удалены 1078 строк кода, добавлены 1345, и это все, что вы поняли, прочитав описание: все остальное как-то расплывчато. Обычно вы подходите к проверкам тщательно, чем и гордитесь, но это явно не тот случай. Внутренне укоряя себя, вы пишете пресловутый комментарий LGTM 👍 («По-моему, неплохо») и одобряете пул-реквест. Ну куда столько файлов!
Не до того, у нас ЧП !
В три часа ночи вас вызвали как дежурного разработчика. В продакшене возникла серьезная проблема: в одном из европейских регионов вдруг резко возросла активность пользователей, что привело к переполнению кэша. На такой случай у вас настроена автоматика, но в этот раз она не сработала. Так что понадобится хотфикс. Довольно быстро найдя решение, вы загружаете исправление прямо в продакшен, минуя налаженный процесс код-ревью: в такой ситуации это было оправданно. Однако этот случай привел к появлению нездорового прецедента: ваши коллеги теперь игнорируют правила, если у них возникает какая-то срочная надобность. И это становится нормой, а не исключением.
Ворон ворону глаз не выклюет
После упорных трудов вы наконец закончили работу над функцией, порученной вам в этом спринте. С гордостью и облегчением вы открываете пул-реквест, тщательно наполняете его нужным контентом и отправляете ревьюеру. Проходит день, вы проверяете состояние пул-реквеста и видите: он все еще открыт, хотя у ваших коллег уже все одобрено. Обидно. Внимательно присмотревшись, вы замечаете: двое из ваших коллег — закадычные друзья — взаимно одобряют код друг друга без всякой проверки и поступают так довольно давно. Такое поведение не может не влиять на мнение разработчиков о код-ревью, хотя в этой ситуации дело вовсе не в нем. Просто когда в процессе вдруг выявляется вина отдельных членов команды, люди все равно винят во всем код-ревью.
Одобрение через чат
В вашей команде н��давно появилась новая политика код-ревью: любой пул-реквест должен одобрить как минимум один ревьюер. Однако вы видите, что большинство запросов одобряются буквально за пять минут. А ведь в команде всего одиннадцать разработчиков. Как же они успевают делать по несколько проверок в день в дополнение к основной работе? Вскоре все проясняется: кто-то придумал, как обойти процесс при помощи мессенджера. Как только кто-то открывает пул-реквест, он сразу же пишет об этом в чат, а кто-то из коллег, прочитав сообщение, сразу заходит в пул-реквест и одобряет его без проверки. Причем одним из таких «серийных одобрителей» оказался QA-инженер. В итоге, несмотря на установленную политику, код-ревью работают совсем не так, как должны.
Ленивые код-ревью бесполезны. Во всех этих случаях болевые точки очевидны: избирательный подход к процессу, одобрение без надлежащей проверки (обычная лень или осознанный поиск обходных путей) — все это может привести к недовольству, напряженности в отношениях и стрессу. А если техлид или менеджер игнорирует или даже потворствует такому поведению членов команды, положение становится еще хуже.
Злобные код-ревью
Злобные код-ревью, когда автор подвергается ненужным нападкам со стороны ревьюера, могут нанести команде непоправимый ущерб. Если вы когда-либо получали чрезмерно негативные комментарии, нацеленные скорее лично на вас, чем на проверяемый код, то вы понимаете, о чем я. И это именно то, что больше всего страшит разработ��иков. Давайте рассмотрим несколько типичных случаев.
Я не выбираю выражений
В публичном репозитории CSS Framework — проекта с открытым исходным кодом — в одном из пул-реквестов я обнаружила весьма характерный комментарий (рис. 7.1). Я привожу его как есть, без купюр (за исключением имени ревьюера и легкой цензуры). Заметьте: это реальный репозиторий из интернета.
исходным кодом)
Если открывший этот пул-реквест человек впервые участвовал в проекте с открытым исходным кодом, он вряд ли когда-то решится на повторную попытку. Уж точно не в этом проекте, где почувствовал себя нежеланным гостем. Ревьюер же дал нам яркий пример того, как не нужно писать комментарии. Есть лучшие, более конструктивные способы донести до автора свою точку зрения.
Ты здесь чужой
Недавно вы стали членом команды разработчиков. Вы далеко не новичок в разработке ПО, но еще не освоились на новом месте. Нервничая, но с воодушевлением вы стремитесь внести свой вклад в кодовую базу, привыкнуть к проекту и коллегам по команде. И вот после долгих усилий и тщательной проверки вы открываете свой первый пул-реквест, а через несколько часов на вас обрушивается шквал комментариев с подробным списком того, что нужно исправить. Куча субъективных замечаний с упоминанием «глупых» ошибок и других схожих эпитетов. Вы чувствуете обиду. Не на такой прием вы надеялись.
Теперь поставьте на это место джуниор-разработчика или же человека, который только что начал работать в ИТ. Как повлияет на него этот случай? Какое мнение о код-ревью у него сложится? Захочет ли он остаться в такой команде? Работать в компании, где такое общение в норме? Вообще в индустрии? Я думаю, вы знаете ответ.
Код-ревью не должны быть злобными. Бестактные ревьюеры, не способные давать конструктивные отзывы, могут нанести команде непоправимый вред.
Код-ревью мутирующего кода
Такие код-ревью — непосильный труд для ревьюеров, поскольку код, с которого началась проверка, к ее концу изменяется до неузнаваемости. Как правило, это значительно затягивает процесс, что сильно разочаровывает и создает напряжение.
Штабели пул-реквестов
Штабель образуется, когда один запрос создается на основе другого, созданного на основе третьего (и так далее). При этом ревьюеру трудно понять первоначальные намерения автора и контекст, а значит, сложно проверить код. Нужно ли принимать во внимание доработки, сделанные в других пул-реквестах? Как мне писать комментарии, если они лишь частично относятся к данному изменению? А что, если часть запросов написана другим автором? Мне просто упомянуть его в комментариях или пригласить в этот пул-реквест? Это просто невыносимо.
Подвижная мишень
Вам направили «простенький» пул-реквест. Прекрасно. Вы не торопитесь, почти дошли до конца проверки, и тут неожиданно автор делает новый коммит, а значит, придется все начинать сначала. К счастью, новые изменения небольшие: лишь несколько дополнений. Вы снова просматриваете измененные файлы, но в тот момент, когда вы уже почти нажали на кнопку «Одобрить», вы замечаете, что статус пул-реквеста опять изменился на «Готово к проверке» (Ready for review). В чем дело? Все верно: автор опять что-то изменил. И долго так будет продолжаться? Зачем вообще открывать пул-реквест, если работа еще не закончена? Зачем мне тратить время на многократный просмотр одного и того же кода?
Пул-реквесты должны быть готовыми к код-ревью. Эти два примера показывают, как тяжело приходится ревьюеру, когда запрос не готов к проверке, когда невозможно понять, для чего он вообще создан. Проверка таких запросов нерациональна, не говоря уже о том, что вы, как автор, раздражаете своих коллег.
Жестко регламентированные код-ревью
Полная противоположность описанным выше — код-ревью, явно перегруженные требованиями, особенно если не используется автоматика. Если процесс требует множества действий, в том числе выполняемых вручную, недостаточно гибок, неоправданно сложен или чрезмерно строг, разработчики будут стараться обойти его. А значит, его, казалось бы, сильная сторона достаточно быстро станет его слабым местом.
Вы выполнили пункты Е, К, Л, М, Н? И не забудьте про П, Р, С, Т
В одной из моих прошлых команд процесс код-ревью был далек от идеала. Прежде чем загрузить код в продакшен, требовалось сначала вручную запустить несколько скриптов, чтобы подтвердить прохождение проверок (и отметить это в пул-реквесте). Затем в запрос нужно было добавить несколько ссылок: на промежуточную среду, где можно было увидеть код в действии, на логи из этой среды, а также на соответствующий рабочий тикет (поскольку система контроля версий была несовместима с системой управления задачами и работала отдельно). После этого ревьюер приступал к проверке. Но если требовалось что-то изменить, все эти действия приходилось выполнять заново и каждый раз вручную обновлять все эти ссылки. А иногда сервис, который их генерировал, выдавал ошибки, падал или подвисал. Он просто не справлялся с нагрузкой, поскольку обслуживал более ста человек. Как вы догадываетесь, мы ненавидели этот процесс.
На мне сошелся клином свет
На другой моей должности в небольшой компании процесс код-ревью был почти идеальным. До поступления в продакшен код проходил через тестовую и промежуточную среды. Проверки качества были в основном автоматическими, но требовалось одобрение со стороны отдела QA. И все было хорошо, пока не появилось новое правило: все должно было обязательно одобряться проджект-менеджером. То есть для интеграции новой функции в тестовую среду мой пул-реквест должен был одобрить кто-то из разработчиков и вдобавок проджект-менеджер, затем, после тестов, для перехода в промежуточную среду — QA-инженер и снова проджект-менеджер и, наконец, для загрузки в продакшен — технический директор и опять-таки проджект-менеджер.
На первый взгляд, все выглядит очень надежно. На деле же наша команда зависела от проджект-менеджера, наличия у него времени и готовности что-то одобрить. От этого и без того громоздкая и долгая процедура затягивалась еще больше.
Код-ревью должно органично вписываться в остальной рабочий процесс. Если проверка еще не началась, а вы уже вязнете в связанных с ней операциях (мой первый пример), что-то явно идет не так. Если для загрузки обновления вам нужно получить несколько одобрений, собрать подписи, да еще и пройти несколько проверок (мой второй пример), люди, без всяких сомнений, устанут от этой бюрократии. Кроме того, чем дольше задержка с выпуском новых функций, тем хуже для вашей компании, ее репутации. А в самом печальном случае люди начнут искать обходные пути, что плохо скажется и на команде, и на кодовой базе.
Так что же делать?
Если, читая эти примеры, вы с сожалением кивали в знак согласия, то вы, скорее всего, уже пострадали от неэффективных и плохо организованных код-ревью. Хорошая новость в том, что многие из этих болевых точек обычно связаны с проблемами общего характера: отсутствием взаимопонимания между сотрудниками, незнанием процесса проверки и обязанностей его участников, недостаточной автоматизацией и неспособностью наладить взаимодействие через код-ревью. О том, как решить эти проблемы, мы говорили в предыдущей части книги. В следующих же главах нас ждут более сложные вопросы. Давайте посмотрим, как сделать процесс обзора кода еще лучше.
Итоги
С течением времени в процессе код-ревью могут появляться сложные проблемы.
Необходимо активно противодействовать превращению хороших код-ревью в ленивые, злобные, неуправляемые или же слишком сложные. Иначе со временем они станут очень, очень плохими.
В этой статье вы ознакомились с главой из книги Браганца Э. «По-моему, неплохо. Конструктивные код-ревью», которую уже можно предзаказать на нашем сайте со скидкой 35% по промокоду - Предзаказ.
