Comments 42
Спасибо, интересный опыт
Реакция на сообщения в мессенджерах куда выше. И было решено подключить бота к нашему мессенджеру.
Что используете в качестве мессенджера?
Исследование привело в Github, где у нас заведена отдельная команда ревьюеров. Эта команда не обновлялась несколько лет. С тех пор количество сотрудников увеличилось в два раза, но никто из новичков в команду ревьюеров не входил. Иначе говоря, половина команды вообще не участвовала в код-ревью! Исправили это досадное недоразумение.— Вася, ты теперь будешь в команде код-ревью.
— ??? (Что это, повышение? Уже пару лет спокойно работаю, ни о каких ревью разговора не было. Ревьюят же только здешние старожилы — по совместительству ценные ключевые сотрудники, могут себе позволить. А я всего лишь рядовой программист...)
— Будешь делать всё то же, что и раньше, только теперь ещё и ревьюить по 2 пулл-реквеста в день на 10 экранов каждый из рандомных частей проекта. И только попробуй не уложиться в 24 часа.
— Окееей…
Что по поводу 24 часов (а точнее 9h) — это договоренность, которой мы достигли всей командой. Мы вместе пришли к тому, что нам комфортнее будет проводить код-ревью за данное время.
В то же время, мы не заставляем обязательно укладываться за один день. Цитирую:
Мы всегда с пониманием относимся к пулл-реквестам, в которых идёт живое, полезное обсуждение, невзирая на сбитую планку в один день.
— Будешь делать всё то же, что и раньше, только теперь ещё и ревьюить по 2 пулл-реквеста в день на 10 экранов каждый из рандомных частей проекта. И только попробуй не уложиться в 24 часа.
Излишне категорично. Кодревью это такая же работа как написание кода или выяснение требований по задачам. Если понадобится, то и целый день можно потратить на ревью.
Кодревью это такая же работа как написание кода
Ну может в каком-нибудь сферическом Яндексе в вакууме это так. Но вот во всех конторах где работал я, ревью рассматривалось как некая дополнительная нагрузка. Оценивался разработчик именно по скорости выполнения задач, а с ревью важность имело только критическое неуспевание (висящие долго пуллреквесты), за которое можно было получить по шапке. В итоге все время перекосы и тяжелые муки выбора — то ли делать свою задачу, с риском задержать какую-то важную фичу, ждущую ревью, то ли ревьюить, а потом оправдываться почему задачу не сделал в срок.
И дело даже не в том что за это как-то особо серьезно ругали, а именно в этом лишнем головняке выбора на что тратить рабочее время.
Например, у нас закрытая задача — это фича, которая находится в продакшене.
Если у тестировщика проблема, ему помогает ее решить команда. Если в ходе деплоя проблема, ее помогает решить команда.
Задача разработчика не равна написать код, задача разработчика решать проблемы. Написаный, но не не отревьювенный и не отттестированный код, который не находится в продакшене — это нерешенная проблема.
Судя по вашему комментарию, у вас не было команды, а был босс и отдельные подчиненный, и это уже проблема менеджмента.
Многие, чтобы не отвлекаться от работы, разбирает свою почту несколько раз за день. Совсем другое дело мессенджеры. Реакция на сообщения в мессенджерах куда выше. И было решено подключить бота к нашему мессенджеру. Бот шлёт оповещения о статусе код-ревью как для автора пулл-реквеста, так и призывает людей поревьюить. Очень удобно.
Отвлекать в мессенджере — мерзко. Повышать культуру работы с почтой, конечно же, намного сложнее.
Вы озвучили одну проблему — новые сотрудники со временем не становились ревьюерами, из-за этого старые имели сверхнагрузку. Остальное не проблемы, а факты и особенности, не влияющие на скорость.
Вопрос: вы изменили скорость, добавили квадриллион напоминалок. Как поменялось качество ревью?
Отвлекать в мессенджере — мерзко. Повышать культуру работы с почтой, конечно же, намного сложнее.
Бот в мессенджере — как одна из опций. Все люди разные, и по-разному строят свою работу. Коллеги, которым удобнее получать сообщения по почте, могут даже не включать оповещения в мессенджере.
Некоторые просматривают PRы непосредственно через GitHub в определенный промежуток времени своего рабочего дня — это нормально. Потому и SLA у нас — один рабочий день.
Как поменялось качество ревью?
Качество ревью довольно сложно измерить. По нашим меркам качество ревью не изменилось. Также мы стараемся по максимуму автоматизировать всевозможные проверки, чтобы ревьюеры фокусировались исключительно на самом важном.
Вопрос: вы изменили скорость, добавили квадриллион напоминалок. Как поменялось качество ревью?
Скажу за себя. Я как ревьювил так и продолжаю. Т.е. от напоминаний хуже делать его не стал, иначе в чём смысл? Таким темпом можно добиться прохождения ревью и за 10 секунд.
Итого: обнаружили что код-ревью занимает долго, оказалось неправильно считали и почему-то этим занимаются не все.
Почему вообще все программисты не могут делать ревью, а не только особо провинившиеся?
Ревьюшница — разработанная в Яндексе система, которая призывает в PR двух рандомных людей с экспертизой в изменяемом коде и с учетом отпусков и отсутствий
Мне кажется очень часто в командах не бывает двух людей с глубоким понятием изменяемого кода, а если они еще рандомно избираются — то их должно быть больше.
То есть возник вопрос сколько человек вообще участвует в разработке продукта и в этой "случайной выборке двух ревьюверов"
Мне кажется очень часто в командах не бывает двух людей с глубоким понятием изменяемого кода
А это как раз одна из проблем, с которой ревью признано бороться. Если в компании много кода, в котором разбирается только написавший его человек, то компания огребет очень много проблем если с этим человеком что-то случится (заболеет, уволится, пресловутый «bus-factor»). Так что ревью — это в том числе и повод членам команды лучше разобраться в других частях кода, который писали не они.
Так же интересно было бы узнать про то, как у вас реализуются пункты 5-9. Автоматизированы ли они? Что используете?
Немного вот об этом:
Ревьюшница — разработанная в Яндексе система, которая призывает в PR двух рандомных людей с экспертизой в изменяемом коде и с учетом отпусков и отсутствий.
Смотрели ли вы сторону open-source решений, таких как Gerrit, например? То же Openstack-сообщество на его основе построило довольно удобное и функциональное воркфлоу. Конечно, одним герритом там не обошлось, рядом есть еще парочка других инструментов (Zuul и Jenkins, например).
Огромный плюс геррита в том, что у него имеется довольно простой API, вокруг которого можно построить довольно удобные CI/CD процессы. Кроме того, геррит позволяет гибко настроить workflow, благодаря использованию дополнительных плагинов и лейблов/оценок.
Так же интересно было бы узнать про то, как у вас реализуются пункты 5-9. Автоматизированы ли они?
Если ответить коротко, то, да автоматизированы. Как только разработчик создал ПР, помимо ревьюшницы приходит ещё один бот, который запускает небольшой пайплайн в который входят:
- Сборка проекта
- Выкладка в тестовом окружении
- Запуск автоматических интеграционных и e2e тестов
- Анализаторы деградаций различных метрик
Отсюда вытекает что именно на ручное тестирование уходит на так много времени. Далее тикет попадает в другой уже релизный пайплайн с похожим набором шагов, но их там гораздо больше.
Что касается используемых для этого решений, то там в основном всё собственного производства.
А вообще это тема для отдельной большой статьи.
Геррит гибче в настройке workflow, геррит гибче и удобнее в ревью кода — например, такая маленькая фича, как возможность выделить кусок кода и оставить комментарий к нему, очень удобна. Я уж не говорю про все остальное.
У вас негативный опыт его использования? Расскажите, пожалуйста.
У нас сейчас в компании используется геррит, в нем есть конечно свои плюсы (vim-гикам понравится, минималистичный интерфейс из 90-х), но из основного:
— Зависит от команды, но на мой взгляд branch-based ревью намного ближе к жизни, чем commit-based. В Геррите вы ревьювите каждый коммит, в Гитхабе вы ревьювите весь ПР, т.е. ветку (и, конечно, все еще остается возможность видеть отдельные коммиты в этой ветке). И новая фича обычно это не один и не два коммита (перед финальным сквошем).
— Гитхаб это же в целом интегрированное решение: репозиторий, ревью и багтрекер вместе. А Геррит идет как надстройка к репозиторию, поэтому интеграция между ними меньше (банально — подсветка ссылок из коммита «Related to PR #123»)
— В проектах на гитхабе зачастую видел хорошо настроенных ботов на проверку код стайла, например. И отображение результатов наглядно бейджиками и цветной циферкой (плюшки, да, но из маленьких мелочей и складывается общее удобство). Прямо автоматически добавляет комменты на строки, где что-то не так. Не копал сильно насчет интеграции подобного в геррит, но есть ощущение, что если бот и есть, то может только финальный «Verified -1» поставить, без комментариев инлайн.
Самое главное что в команде программисты разных уровней, но все в итоге делают ревью, получается все в курсе всего и учатся друг у друга.
У нас:
— code + written unit tests + functional tests
— pre-review 2 чела + team lead
— submit to development tree
— post review 2 чела
— тестировщики и load tests
--…
Вначале общая архитектура, потом разбивается на спринты, в спринтах задачи по 2 дня максимум. Задача включает в себя
— детальный дизайн + ревью (один опытный, второй любой)
— код, юнит тесты, смок тесты — ревью кода (2 человека, один опытный, второй любой)
— задача сливается
— в конце спринта, еще раз смок тесты все для всех задач и сборка спринта
— тестирование спринта у тестеровщиков…
Поэтому кроме собственной группы разработчиков код ревьюит security department (чисто смотрит чтобы не было глупых дыр — хотя скорее это для ихнего успокоения — статистические анализаторы это делают лучше) и наш менеджер продукта.
Могут код еще смотреть ISO и прочие стандарт ревьюверы.
Мы продолжаем отслеживать процесс код-ревью и решать все технические и даже психологические проблемыВот на последнем пункте можно поподробнее? К примеру, что вы делаете, когда люди завалены демотивирующими код-ревью? Демотивируют, потому что код, мягко говоря, «пахнет», а автор сопротивляется исправлять.
- Как сократить время на ревью?
- Давать меньше времени на ревью.
В следующей серии:
- Чем принципмально отличаются этстимации в сторипоинтах от оценки в нормочасах?
- Ни чем.
рецепты эффективного менеджмента
Автоматизация — это отлично. В остальном мы пошли по другому пути. Можно сказать в обратном направлении. Выглядит это так.
Код ревью занимает столько, сколько занимает и "ускорить" его нельзя. Тестировщиков у нас как таковых нет. Доказательность того, что все работает — это один из аспектов ревью. Хотите — читайте код, хотите — предъявляйте тесты, хотите — шагайте через код дебагером. Ваша задача убедить ревьювера, а он будет упираться. Почему? Потому что, за вынос в прод отвечает не автор, а ревьювер. Если вы напороли, разбираться будете вдвоем, но ответственность именно на ревьювере. Это чтобы народ серьезнее к этому процессу относился а не глянул по диагонали, тесты проходят — и +2. Как результат ревью по три десятка итераций вполне норма.
А как сделать так, чтобы ревью не уходило в бесконечность? На то есть менеджеры. У них есть право и обязанность любыми доступными методами, кроме запрещенных в code of conduct, тянуть разработчиков в сторону закрытия историй. У нас это специальные барышни, которые отвечают за набор историй. Сейчас конкретно их две на наш отдел в 30 человек. Могут три раза на дню прийти поспрашивать как дела и что держит. Могут попросить назначить другого ревьювера и т.д. Если ревьювер и автор уперлись, могут пригласить третьего в качестве арбитра. Если застряли на координации, будут носиться между командами и сводить концы. Если что-то поменялось будут обновлять истории и доносить это до инженеров. Это их работа, главным образом персонально-коммуникационная, и справляются они с ней на ура. Опять же не из альтруистических соображений — если ревью зависло на месяц, есть шанс что всю фичу зарубят как невыполнимую и снесут в бэклог. Им тогда объясняться с кучей народа, в том числе директорами и спонсорами. И OKR могут посыпаться.
Кстати, подчиненных инженеров у барышень нет и начальством они формально тоже не являются (разве только между собой). Это для того, чтобы они обслуживали процесс а не рулили им. В противном случае будет "сделай как я говорю или премия минус".
В целом работать намного комфортнее чем в конторе с кучей мотивационных метрик (я и там был в свое время), а впечатление очень положительное. Когда у вас "ревью в 24 часа" и "история в день", фокус неизбежно сползает с работы на удовлетворение системы или ее надувательство. Конкретно пример из "метрической" конторы (каюсь): иногда если ревью сложное, а мне надо еще кучу всего сделать, я его просто рубил его "за несоответствие требованиям", благо всегда можно прицепиться. Автор понимал что я в зашиве и возвращал историю продакту. Они всегда перегружены, так что пару дней есть пока они доберутся. Потому 10 минутный митинг с чтением истории вслух, уточнение одного предложения, подъем ревью, а у меня уже было время код почитать. Так погоняешь ревью кругами — у всех метрики на ура, коллаборация претъ, все счастливы. Только сплошная Кафка какая-то, причем я не про технологию.
Реакция на сообщения в мессенджерах куда выше. И было решено подключить бота к нашему мессенджеру. Бот шлёт оповещения о статусе код-ревью как для автора пулл-реквеста, так и призывает людей поревьюить. Очень удобно.
А какой у вас мессенджер?
У меня подобного опыта не очень много, но периодически какие-то задачи с коллегами решаем на пару. При этом код-ревью проходит довольно быстро (ревьювит 1 человек т.к. над задачей работали 2) и субъективно проблем с таким кодом потом меньше.
А кто у вас мерджит PR в случае конфликтов — разработчик или ревьювер?
Не думали о том, что в случае конфликта мерджить должен ревьювер?
Очень хорошо мотивирует рассматривать PR вовремя.
Разве что вы имеете ввиду, что ПР быстро устаревает и повышется риск возникновения конфликтов. Ну да, это так. Но, в нашем случае разработчику тоже не интересно, чтобы его тикет долго висел в ревью, поэтому он всё равно не исключается из процесса и наблюдает за статусом своих ПР.
Разве разработчики не делают rebase целевой ветки (master/development) в свой PR перед сабмитом на ревью? Тогда свежий PR гарантированно мерджится без конфликтов.
В настоящее время, когда девопс борятся за лишние минуты и секунды turnaround-а, ждать ревью сутки — какая-то средневековая дикость.
А минуты и секунды — это вы в утопии какой-то, где девелоперы пинают болт, что готовы ревьювить код в ту же секунду, как прилетела нотификашка. Не говоря уж про то, что может быть такой код, что в нем только час-другой разобраться надо.
С другой стороны, всё нормально, давно настало время, когда разработка суть обычный промышленный процесс, а гордые знатоки 100500 аббревиатур — банальные рабочие за станком. Прошло время не только романтики, но и ремесленничества.
Все наши шаги привели к желаемому результату: более 92% пулл-реквестов стали удовлетворять нашему SLA!
Сразу возникает вопрос — а чем это хорошо? На основании чего была отвергнута гипотеза о том, что выполнение данной SLA снижает скорость доставки и качество доставленных фич?
Как сократить код-ревью с двух недель до нескольких часов. Опыт команды Яндекс.Маркета