Как проверять чужой код и не бесить коллег
Анонимный программист опубликовал на блог-площадке Medium свои размышления о том, как ускорить процедуру проверки кода и сделать её безболезненной для всех учестников процесса. dev.by приводит полный перевод статьи.
На дворе 2018 год, и все продвинутые разработчики хранят исходный код на GitHub или GitLab и используют какой-нибудь модный git-процесс разработки вроде вот этого. Они верят в концепцию непрерывной интеграции и доставки CI/CD, у них ни дня не обходится без крутой новой фичи, а словосочетание «пулл реквест» входит в топ-10 выражений, которые неизменно присутствуют в диалогах между разработчиками. Среди остальных девяти, скорее всего, «что за…», «это моя еда!!!», «кто на обед?» и «жду ревью».
Пулл реквест
Продакт-менеджеру приходит в голову блестящая идея: «Надо добавить в наше приложение больше гифок!» Естественно, проходит недолгое совещание, менеджер получает бонус за эту потрясающую идею, а разработчики — ещё одну классную карточку с подробным ТЗ «добавить гифки». Весьма исчерпывающе.
Задание получили, за работу. Делаем форк от ветки master, пишем безупречно чистый код, отправляем на GitHub и ждём чуда.
Дальше будет нужно сравнить изменения с основной веткой и сделать новый пулл реквест. В два клика запрос открыт, и вы надеетесь, что коллеги, которых вы и так недолюбливаете, не будут браться за него и просматривать код.
Ревью кода
Супер. Код приняли, всё прошло как по маслу, новая фича готова и ожидает ревью команды. Но потом вы смотрите на страницу пулл-реквеста в GitHub…
23 открытых пулл реквеста
Это закончится нескоро…
В реальности средняя продолжительность жизни пулл-реквесты — от создания до слияния — очень много дней. Для agile-команд это отличается от нормальной практики. Каждый день, пока код ждёт ревью, где-то без гифок плачут пользователи. Но нельзя же лишать людей гифок!
Итак, теперь мы знаем, как плохо слишком долго ждать ревью. Что можно сделать, чтобы его ускорить?
Неблокирующие код-ревью
Знакомьтесь, Чад. Чад — разработчик программного обеспечения, и он только что открыл пулл-реквест. Но он не стал ждать, пока кто-то проверит изменения. Вместо этого он решил назначить ревьюером себя и самостоятельно влить изменения, после чего они отправятся на тестирование и продолжат развёртываться. Ловко, да?
Он что, добавляет непроверенные изменения???
Так и есть.
Потому что Чад «гибкий» до мозга костей, и он знает, что выпустить фичу для пользователя гораздо важнее, чем заморачиваться со стилем кода. И за это получает звание «Короля гифок».
Спустя какое-то время один из коллег Чада таки принимается за упомянутый пулл-реквест. Он ворчит и поглаживает свою длинную бороду. Он всей душой ненавидит вспомогательные классы, и правильно делает. А потом смотрит на них и пишет: «У меня сейчас кровь из глаз польётся. Пожалуйста, уберите их».
Чад читает комментарий, драматично вздыхает и помечает себе, что вспомогательные классы нужно удалить в следующей итерации.
Аджайл как он есть.
Итак, вы застряли на слиянии, и код ещё не прошёл ревью. Но если вы так и не убедились в том, что важнее выпустить функцию, чем проверять код на соответствие стандартам, вот ещё несколько причин в пользу этого.
- Ревью кода не настолько существенный процесс, как кажется
Вспомните изменения, которые вы вносили или проверяли. В 90 процентах случаев они касались стиля кода и различных формальностей. Вряд ли вам попадалось много комментариев в духе «из-за этого куска кода будет утечка памяти, приложение рухнет и наступит конец света», потому что ревью — это когда кто-то просматривает код в промежутке между другими делами, хотя должен был бы вдумчиво его анализировать;
- Чем дольше пулл-реквест ожидает просмотра, тем выше вероятность того, что он получит статус deprecated
Я много раз видел, как запросы обсуждали настолько долго, что в конце концов их просто закрывали, потому что фича была больше не нужна. Если компания хочет быстро развиваться, она должна быстро выпускать функционал. Если команда на это неспособна, проблемы будут у всей компании. Потеряли время — потеряли возможность продвинуть компанию;
- Моральное давление непроверенных пулл-реквестов
Ожидающий просмотра код будут снова и снова обсуждать на ежедневных митингах, пока в какой-то момент времени назначенному ревьюеру это не надоест настолько, что всё же проверит код. Но только потому, что это рано или поздно придётся сделать, а не потому, что он свято верит в силу код ревью.
Возрождение код ревью
Пока что мы говорили о том, почему блокировать пулл-реквесты — плохо, а использовать неблокирующие код-ревью — хорошо. С этим разобрались.
Но что можно сделать, чтобы стимулировать этот процесс?
Отличный вопрос, потому что он поможет значительно упростить работу.
- Парный просмотр кода
Вместо того, чтобы просматривать код и пассивно-агрессивно комментировать его на GitHub, контрибьютор и ревьюер могут собраться и вместе пройтись по предлагаемым изменениям. Это в разы эффективнее: возможно, в жизни этот бородатый коллега окажется не только злым занудой, но и вполне интересным собеседником, который любит котов и коллекционирует шляпы, а вспомогательные классы терпеть не может потому, что не признаёт «ленивую» разработку;
- Закладывайте время на проверку кода, когда планируете задачи
Некоторое количество времени в неделю нужно выделять на тщательный просмотр кода или же подачу запросов на добавление изменений.
В заключение, если команда блокирует проверку пулл реквестов, посмотрите, мешает ли это вашему рабочему процессу. Если ответ «да», используйте неблокирующие пулл-реквесты.