Как проверять чужой код и не бесить коллег

12 комментариев
Как проверять чужой код и не бесить коллег

Анонимный программист опубликовал на блог-площадке Medium свои размышления о том, как ускорить процедуру проверки кода и сделать её безболезненной для всех участников процесса. dev.by приводит полный перевод статьи.

Читать далее

Иллюстрация: Depositphotos

На дворе 2018 год, и все продвинутые разработчики хранят исходный код на GitHub или GitLab и используют какой-нибудь модный git-процесс разработки вроде вот этого. Они верят в концепцию непрерывной интеграции и доставки CI/CD, у них ни дня не обходится без крутой новой фичи, а словосочетание «пулл реквест» входит в топ-10 выражений, которые неизменно присутствуют в диалогах между разработчиками. Среди остальных девяти, скорее всего, «что за…», «это моя еда!!!», «кто на обед?» и «жду ревью».

Пулл реквест

Продакт-менеджеру приходит в голову блестящая идея: «Надо добавить в наше приложение больше гифок!» Естественно, проходит недолгое совещание, менеджер получает бонус за эту потрясающую идею, а разработчики — ещё одну классную карточку с подробным ТЗ «добавить гифки». Весьма исчерпывающе.

Задание получили, за работу. Делаем форк от ветки master, пишем безупречно чистый код, отправляем на GitHub и ждём чуда.

Скриншот: Medium

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

Ревью кода

Супер. Код приняли, всё прошло как по маслу, новая фича готова и ожидает ревью команды. Но потом вы смотрите на страницу пулл-реквеста в GitHub…

23 открытых пулл реквеста

Это закончится нескоро…

В реальности средняя продолжительность жизни пулл-реквесты — от создания до слияния — очень много дней. Для agile-команд это отличается от нормальной практики. Каждый день, пока код ждёт ревью, где-то без гифок плачут пользователи. Но нельзя же лишать людей гифок!

Итак, теперь мы знаем, как плохо слишком долго ждать ревью. Что можно сделать, чтобы его ускорить?

Неблокирующие код-ревью

Знакомьтесь, Чад. Чад — разработчик программного обеспечения, и он только что открыл пулл-реквест. Но он не стал ждать, пока кто-то проверит изменения. Вместо этого он решил назначить ревьюером себя и самостоятельно влить изменения, после чего они отправятся на тестирование и продолжат развёртываться. Ловко, да?

Он что, добавляет непроверенные изменения???

Так и есть.

Потому что Чад «гибкий» до мозга костей, и он знает, что выпустить фичу для пользователя гораздо важнее, чем заморачиваться со стилем кода. И за это получает звание «Короля гифок».

Спустя какое-то время один из коллег Чада таки принимается за упомянутый пулл-реквест. Он ворчит и поглаживает свою длинную бороду. Он всей душой ненавидит вспомогательные классы, и правильно делает. А потом смотрит на них и пишет: «У меня сейчас кровь из глаз польётся. Пожалуйста, уберите их».

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

Аджайл как он есть.

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

  • Ревью кода не настолько существенный процесс, как кажется
    Вспомните изменения, которые вы вносили или проверяли. В 90 процентах случаев они касались стиля кода и различных формальностей. Вряд ли вам попадалось много комментариев в духе «из-за этого куска кода будет утечка памяти, приложение рухнет и наступит конец света», потому что ревью — это когда кто-то просматривает код в промежутке между другими делами, хотя должен был бы вдумчиво его анализировать;
     
  • Чем дольше пулл-реквест ожидает просмотра, тем выше вероятность того, что он получит статус deprecated
    Я много раз видел, как запросы обсуждали настолько долго, что в конце концов их просто закрывали, потому что фича была больше не нужна. Если компания хочет быстро развиваться, она должна быстро выпускать функционал. Если команда на это неспособна, проблемы будут у всей компании. Потеряли время — потеряли возможность продвинуть компанию;
     
  • Моральное давление непроверенных пулл-реквестов
    Ожидающий просмотра код будут снова и снова обсуждать на ежедневных митингах, пока в какой-то момент времени назначенному ревьюеру это не надоест настолько, что всё же проверит код. Но только потому, что это рано или поздно придётся сделать, а не потому, что он свято верит в силу код ревью.

Возрождение код ревью

Пока что мы говорили о том, почему блокировать пулл-реквесты — плохо, а использовать неблокирующие код-ревью — хорошо. С этим разобрались.

Но что можно сделать, чтобы стимулировать этот процесс?

Отличный вопрос, потому что он поможет значительно упростить работу.

  • Парный просмотр кода
    Вместо того, чтобы просматривать код и пассивно-агрессивно комментировать его на GitHub, контрибьютор и ревьюер могут собраться и вместе пройтись по предлагаемым изменениям. Это в разы эффективнее: возможно, в жизни этот бородатый коллега окажется не только злым занудой, но и вполне интересным собеседником, который любит котов и коллекционирует шляпы, а вспомогательные классы терпеть не может потому, что не признаёт «ленивую» разработку;
     
  • Закладывайте время на проверку кода, когда планируете задачи
    Некоторое количество времени в неделю нужно выделять на тщательный просмотр кода или же подачу запросов на добавление изменений.

В заключение, если команда блокирует проверку пулл реквестов, посмотрите, мешает ли это вашему рабочему процессу. Если ответ «да», используйте неблокирующие пулл-реквесты.

Хотите сообщить важную новость?

Пишите в наш Телеграм

Горячие события

Конкурс EY Entrepreneur Of The Year 2020
31 мая

Конкурс EY Entrepreneur Of The Year 2020

EMERGE 2020
1 июня — 3 июня

EMERGE 2020

Вебинар «Советы от рекрутеров: как найти квалифицированную работу в Европе»
4 июня

Вебинар «Советы от рекрутеров: как найти квалифицированную работу в Европе»

Читайте также

«Почему разработчики участвуют в этом цирке?». Хайринг как главный баг ИТ-индустрии
«Почему разработчики участвуют в этом цирке?». Хайринг как главный баг ИТ-индустрии

«Почему разработчики участвуют в этом цирке?». Хайринг как главный баг ИТ-индустрии

Австралийский разработчик и CTO Нейл Сайнсбари в ужасе от того, как собеседуют и принимают на работу технических специалистов. Публикуем перевод его колонки.
18 комментариев
HackerEarth: Go снова назван самым востребованным языком среди программистов
HackerEarth: Go снова назван самым востребованным языком среди программистов

HackerEarth: Go снова назван самым востребованным языком среди программистов

20 вещей, которые я вынес за 20 лет в программировании
20 вещей, которые я вынес за 20 лет в программировании

20 вещей, которые я вынес за 20 лет в программировании

Программист Schibsted Алекс Эвелёф в блоге на Medium рассказал о главных правилах и принципах, которые вывел для себя за многие годы работы в ИТ. Публикуем перевод статьи.
12 комментариев
40 из 60+ не комментируют вообще. Узнали, как профсоюзы идут в ИТ-компании
40 из 60+ не комментируют вообще. Узнали, как профсоюзы идут в ИТ-компании

40 из 60+ не комментируют вообще. Узнали, как профсоюзы идут в ИТ-компании

Первичные профсоюзные организации, по данным ПВТ, созданы в двух десятках компаний-резидентов. «Этот процесс находится ещё в начальной стадии, но в некоторых компаниях профсоюзные организации функционируют уже давно», — рассказали dev.by в администрации Парка по итогам круглого стола с участием Федерации профсоюзов Беларуси.
35 комментариев

Обсуждение

5

Да, давайте удвоим работу для QA, а то без дела сидят, бедолаги!
Как по мне, так просто надо серьёзнее относится к Code Review и не позволять пулл реквестам жить неделями.

3

- Насколько по шкале от 1 до 10 вы оцениваете свою серьезность в отношении код ревью?
- На 7.
- С этого дня давайте на 8.

0

"На сколько".

Андрей Митин
Андрей Митин System analyst в CTDEV
3

Имхо, вредная статья.
Ревью не из-за стилей делают. То, что в 90% случаев замечания касаются стилей и прочих формальностей, не значит, что только это и проверялось.
А вообще можно взглянуть на SonarCube, например, в качестве помощника.

saz
saz
1

>У меня сейчас кровь из глаз польётся. Пожалуйста, уберите их
Тех, кто использует подобные аргументы вместо конструктивной критики надо просто увольнять.
И очень простое правило - не нужно смотреть на дизайн кода на ревью, если это не ревью архитектуры. 2 или 3 класса сделал программист разницы нет, если задача решена корректно.

0

не передергивай. В реальности нормальные причины все пишут

saz
saz
0

В статье конкретный пример, когда эмоции есть, аргументов нет. Лично я считаю, что нельзя давать код на ревью людям, у которых soft skills ниже плинтуса и которые не могут говорить исключительно по делу. Такие эмоции можно позволить лишь с друзьями. Да и то, не стоит. Потому что вреда от такого ревью будет больше чем пользы.

-2

Статья - ахинея. Чтоб этот "анонимный программист" всю жизнь фиксил баги после "гибких разработчиков" которые игнорят ревью.

"Проблемы" с ревью описанные в статье надуманны. В случае адекватной команды все решается коммуникацией. В случае неадкватной - вы действительно хотите чтобы ревью игнорились?

0

Спорная и нормальная статья, просто у многих программистов завышенное чувство собственной важности, и нет в ИТ иерархии ,вот они и выплескивают свои комплексы в код ревью.

1

Все должно быть в меру. Например, проект сделан на ООП, и тут приходит новый смузи паренек, начитавшийся умных книг, и нахлебавшийся бесплатного кофе на различного вида конференциях, и начинает ФП. Или иерархии классов городить по 6 наследований в одном пулл реквесте.

0

> https://cdn-images-1.medium.com/max/533/1*bLrilityrp6Ns1Q-Rs-ZUg.gif
тоже так хочу :)

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

конечно, если это не какая-то новая и экспериментальная фича, то лучше выделить достаточно времени чтобы сразу сделать всё хорошо, ведь технический долг никуда сам не испарится. (;`

0

Это же суть аджайла - гибкость ради быстрых измененния. Скорость важнее качества. Сам принцип об этом. Качество приорететнее скорости в класицеском ватерфоле.

Спасибо! 

Получать рассылки dev.by про белорусское ИТ

Что-то пошло не так. Попробуйте позже