![](https://habrastorage.org/webt/6o/7q/zs/6o7qzskmsa3znsxihdvy6bzjbhi.png)
Привет, читатель! Меня зовут Илья Агеев, я работаю в Badoo уже почти семь лет. Текущая моя должность называется Engineering Director Quality Assurance (директор по контролю качества), и по факту я занимаюсь много чем в компании: контроль качества, релиз-инжиниринг, процессы и методология разработки и даже немного информационная безопасность. Одна из сфер, которая входит в зону моей ответственности, — ревью кода. Об истории и результате нашего процесса (которым стал опенсорсный codeisok) я сегодня и расскажу.
Статья содержит историческое описание, технического сравнения различных инструментов для ревью кода в ней нет. Тем не менее я даю примеры таких инструментов, и если вы заинтересуетесь вопросом, то вполне можете попробовать самостоятельно установить и сравнить тулзы.Давным-давно в Badoo не было code review. Практически. Первой попыткой обмена информацией об изменённом коде была простая рассылка изменений на почту. К слову, и Git тогда не использовалась.
Летом 2011 года, когда я только пришёл в компанию, для хранения версий кода использовалась SVN. Бывалые рассказывали, что до этого все работали в CVS и что была одна попытка перейти на Mercurial, но она провалилась. Почему — никто не помнит. Я подозреваю, что дело в том, что, когда решили попробовать Mercurial, особой надобности в переходе на другую VCS не было. Всё и так всех устраивало: работает, и ладно. А модный инструмент ради моды никому не нужен.
![](https://habrastorage.org/webt/d4/xn/sh/d4xnshcaanmuhyhcgygyudjpeqa.gif)
В любом случае я те времена не застал, и моё знакомство с проектом тесно связано с SVN. Разработка велась в одной ветке — trunk. Иногда сторонние экспериментальные вещи делались в отдельных ветках SVN с последующими мучительными попытками смёржить то, что получилось, в общую ветку. Оно и неудивительно: SVN — не самая идеальная вещь для работы в ветках.
Флоу работы выглядел примерно так: берём голову trunk, делаем изменения в коде, коммитим (SVN принимает коммиты сразу на сервер, в общий код) и надеемся, что всё будет хорошо. Запоминаем номер ревизии, полученный от SVN, и прописываем его в тикет, чтобы в выкладке было понятно, какие задачи в неё попали. Очевидно, что, если номер ревизии в тикете меньше номера выложенной на продакшн ревизии, значит, код этой задачи есть в истории коммитов.
После коммита дифф с изменениями отправлялся на почту, в общую рассылку. Благодаря этому те, кто хотел ознакомиться с изменениями кода, могли это сделать. Естественно, никакого упорядоченного процесса code review не было.
Git
Мне как руководителю QA-отдела, только пришедшему в проект и озабоченному правильным построением процесса, сразу стало понятно, что дальше так жить нельзя. Код, который разрабатывается в одной ветке, который сложно анализировать на наличие проблем и ошибок, который тяжело раскладывать и невозможно откатывать, хорошему тестированию совершенно не способствует.
![](https://habrastorage.org/webt/eg/fu/yr/egfuyrnnnpfnronpalluk8c4lhu.gif)
Соответственно, первое, что следует сделать, — отделить мух от котлет. Результатом стал переход на Git и идеологию фичебранчей со всеми вытекающими преимуществами такого подхода. Подробно об этом переходе я рассказывал на РИТ++ в 2012 году в парном докладе с Юрием Насретдиновым.
При этом нужно было как-то отслеживать изменения, сделанные в коде для задачи и тем более в ветке релиза. Чтобы понимать, что сделано, как это тестировать, выкладывать, находить корни ошибок и т. д., мы сделали две вещи:
- Оставили «старый» механизм оповещения об изменениях в виде письма на почту. К слову, этот подход используется до сих пор, многие наши инженеры находят это удобным: можно в любой момент времени в почте найти изменения, сделанные конкретным человеком в конкретных участках кода и в конкретной задаче. Сегодня в Badoo этот механизм работает параллельно с обычным процессом code review.
- Я установил просмотрщик кода в центральном Git-хранилище, где можно было видеть ветки в репозитории, коммиты, файлы и т. д. Когда код задачи попадал в общий репозиторий, Git-хуком прописывалась ссылка на изменения в этот веб-интерфейс, чтобы каждый интересующийся мог пройти по ссылке и увидеть изменения.
В тот момент наиболее подходящим для этой цели инструментом мне показался GitPHP, практически полный клон cgit. Инструмент мне понравился потому что его минималистичный интерфейс давал возможность делать всё, что нам требовалось, а открытый исходный код на PHP позволял нам менять/ добавлять что-то по мере необходимости.
Домен инструмента http://gitphp.org/, к сожалению, на сегодняшний день мёртв. Репозиторий на GitHub также удалён — видимо, автор больше не хочет развивать и поддерживать инструмент. Но мы помним https://github.com/xiphux/gitphp и очень благодарны автору — Chris Han.
Мы любим и уважаем open-source, и в GitPHP, как и во многих других инструментах, с которыми мы работаем, сделали несколько улучшений. Вот в этой статье от ноября 2013 года, к примеру, мы рассказывали, как оптимизировали скорость работы инструмента и как Chris Han принял их в основной код.
Code review
На Git перешли, процесс настроили. Что дальше? Ещё одним моментом, стратегически влияющим на многие вещи в разработке, является ревью кода. Процесс, имеющий массу плюсов (минусы тоже есть, как-нибудь я опишу их в отдельной статье), в нашей компании построен не был.
![](https://habrastorage.org/webt/8x/c8/an/8xc8anuuht2khbx3nw-od56dtw0.gif)
Мы стали тестировать разные инструменты для организации code review, делали пробные инсталляции и изучали вопрос. В первую очередь мы хотели всё организовать для основного репозитория Badoo с PHP-кодом, а уже потом, если всё будет хорошо, распространить на всю компанию.
Важнейшими требованиями, которые мы предъявляли к процессу и инструменту, на тот момент были:
- Возможность хостить инструмент внутри компании. У нас есть несколько разработок, которые являются интеллектуальными ноу-хау, да и с точки зрения здравого смысла не очень хочется, чтобы исходники могли утечь в Сеть.
- Чтобы код можно было смотреть/ ревьюить/ проверять сразу. И сразу давать фидбек. Не ждать, пока разработчик формально укажет, что всё готово, как это делается с пулл-реквестами в GitHub, например. Тем более очень много базовых проверок мы делаем автоматически на момент пуша кода в общий репозиторий.
- Процесс внедрения должен быть прозрачным, плавным и не должен мешать текущим задачам, которые уже на конвейере — чтобы никто не заметил особых изменений, а, напротив, люди получили новые возможности в случае необходимости.
- Интеграция с Jira, которая на тот момент уже широко использовалась в компании.
На данном этапе необходимо пояснить два новых термина, которые мы используем и которые отличаются от идеологии пулл-реквестов. Это бранчдифф и бранчлог. По сути, это просто разница в коде между базовой веткой (у нас это master в большинстве случаев) и веткой задачи. Бранчдифф — полный дифф между двумя головами веток. Бранчлог — полный лог коммитов, показывающих разницу между головами двух веток. По мере роста ствола коммитов ветки задачи бранчдиффа и бранчлога могут расти.
Gerrit
Примерно в то же самое время, когда я разбирался с основным репозиторием кода и PHP’шниками, наши сишники озаботились проведением code review в своих репозиториях (их у них много) и самостоятельно решили опробовать Gerrit. Антон Довгаль вот в этой статье подробно рассказывал о том, как это было, какие инструменты ещё рассматривались и какие проблемы хотелось и удалось решить.
Забегая вперёд, скажу, что сегодня Gerrit в Badoo не используется. А на тот момент для других отделов не подходил, так как умел делать ревью только на покоммитные изменения. Ревью кода всей ветки (бранчдиффа) инструмент делать не умел. Честно говоря, я не уверен, что умеет и сейчас. Поэтому от Gerrit как инструмента для ревью кода во всей компании мы отказались.
![](https://habrastorage.org/webt/l8/z0/zr/l8z0zrhccqqiinbanrj5v7u1v4o.gif)
GitLab / GitHub
Изучать инструменты для ревью кода нам тогда помогал руководитель бэк-офиса Борис Чернецов. Боря ставил разные тулзы, показывал, как они работают, предлагал процесс ревью, поддерживаемый этими инструментами, и т. д.
Но ни один из рассматриваемых на тот момент инструментов нам не подошёл (стандартная фраза — оправдание изобретения собственного велосипеда). Насколько я помню, основными проблемами были:
- Многие инструменты не предлагали self-hosted-решений. А для нас это было одним из базовых требований.
- Отсутствие pre-receive-хуков, что отдаляло проверки и удлиняло цикл разработки. Все автоматические проверки можно было делать только постфактум, после создания пулл-реквеста. Так работает GitHub, например, который к тому же ещё и является платным сервисом.
- GitLab был тогда очень сырым и содержал кучу багов, хотя поначалу нам очень понравился. Очень многое из заявленного функционала просто не работало. Сегодня, насколько я знаю, ребята ушли далеко вперёд и хорошо развили проект.
Мы рассматривали ещё несколько инструментов вроде Gitorious и других, но они сильно уступали в функциональности даже GitLab и не предоставляли многих необходимых нам функций.
GitPHP для code review
Разочаровавшись в тех инструментах, что мы сумели найти, мы решили пойти совершенно другим путём. Раз нет инструментов, которые нам подходят на 100%, а правка кода GitPHP не является такой уж сложной задачей, то почему бы не попробовать сделать простейший механизм комментирования кода в GitPHP? Тем более что все остальные необходимые нам вещи хранение кода на сервере, управление доступом Gitosis, привязка к тикетам, просмотр изменений веток и т. д. — уже есть.
На том и порешили.
На создание первой версии у нас ушло совсем немного времени. Николай Крапивный помог нам сделать авторизацию через Atlassian Crowd и простейшую форму комментирования строк кода за пару выходных. И мы поняли, насколько это хорошо и удобно: тыкаешь на ссылку в тикете (ту, что уже там была до этого, к которой все привыкли) — и можешь писать комментарии к коду.
Комментарии отправляются разработчику на имейл и могут помещаться в публичные рассылки. А ещё добавляются в тикет в Jira. Очень удобно. Заморачиваться с созданием отдельных «тикетов для ревью» (как делает Crucible, например), назначением ревьюверов внутри GitPHP (ревьювер — тот, на кого перевели тикет после разработки в Jira), отдельным статусом ревью и т. д. не нужно. Есть комментарии, переоткрыли тикет — значит, код надо исправлять. Если всё хорошо, просто переводим тикет дальше.
Эта простая схема настолько органично вписалась в проект и так понравилась разработчикам, что её легко и быстро подхватили и стали использовать.
Crucible и FishEye
Но несмотря на то, что бекенд-разработчики быстро подхватили идею и стали широко использовать инструмент, а некоторые даже помогали его развивать, не всё было идеально.
Мобильные разработчики поначалу сильно сопротивлялись и отказывались использовать инструмент. Отговорки были разные: начиная с того, что интерфейс не нравится, заканчивая тем, что в нём нет каких-то остро необходимых им штук.
Мы несколько раз делали обзоры инструментов для code review, существующих на рынке, пытаясь найти что-нибудь подходящее в надежде на улучшение ситуации. Инструменты действительно развиваются, в них добавляется много полезных и не очень фич, появляются новые. Но и проблемы со всем этим возникают новые.
Так, в наших исследованиях участвовали следующие инструменты для ревью: Upsource, Crucible, GitLab, Phabricator, Review Board, Kallithea и несколько других. К сожалению, идеального инструмента среди них так и не нашлось.
Несмотря на это, мы всё-таки поставили Crucible параллельно с GitPHP и целый год сравнивали работу этих двух инструментов. Crucible использовали многие мобильные разработчики в своих предыдущих проектах и давали положительные отзывы. Интеграция с Jira у него была одной из лучших, что неудивительно — его тоже делает Atlassian.
Основная же проблема Crucible, как оказалось, производительность. Нам даже пришлось сильно урезать его функционал отключением нескольких фич.
Мы пробовали разное железо, разные оптимизации, рекомендуемые разработчиками, но так ничего и не добились. Самое быстрое, что нам удалось выжать из этого инструмента, — десять минут до появления кода в инструменте после пуша. А в случае с некоторыми задачами, зависимость которых была непонятна, и по паре часов приходилось ждать. После увеличения количества репозиториев и коммитов скорость падала нелинейно. Для нашей динамики разработки и поставки фич это не подходило.
Индекс, который Crucible строит и хранит внутри и который служит базой для показа диффов, логов и блеймов, строится невообразимо медленно. Мы даже разные движки баз данных пробовали. Иногда по непонятным причинам Crucible начинал «подвешивать» и Jira. Лечилось это рестартом обеих систем.
Ну и в итоге получилось так, что GitPHP и работает в разы быстрее, и многие хотелки по запросам мобильных разработчиков мы успели добавить, и у Crucible масса своих проблем. Соответственно, поигравшись год с Crucible, мы приняли волевое решение внедрить GitPHP и у мобильных разработчиков тоже.
Сегодня инструмент используется в нашей компании как стандарт, во всех отделах. И сишники, которые игрались с Gerrit, используют этот инструмент, и мобильные разработчики, и PHP’шники, и JS’ники с верстальщиками — в общем, все-все-все.
Codeisok
Конечно, с тех пор как мы начали работать с GitPHP, много воды утекло, и мы практически до неузнаваемости изменили оригинальный инструмент. Он получил новый функционал, новый дизайн, кучу дополнительных возможностей, оптимизаций и улучшений.
![](https://habrastorage.org/webt/p5/sx/ma/p5sxmahdbhuf0dhrwyrghzb4psa.gif)
Мы добавили несколько методов авторизации: не только через Jira и Crowd, но ещё и через Redmine, к примеру. Мы изменили подсветку синтаксиса и полностью переписали режим side-by-side. Мы избавились от Gitosis и реализовали управление репозиториями, пользователями и их доступом через веб-интерфейс — легко и удобно (Саша и Женя, спасибо). Мы добавили фильтры по файлам (Саша, спасибо), расширениям, изменениям в диффах и ревью. Мы сделали установку инструмента лёгкой, обернув всё в докер. Мы полностью поменяли дизайн. Ну и про саму возможность делать code review не забываем — она тоже эволюционировала и стала гораздо удобнее.
В общем, инструмент стал совсем другим, поэтому мы дали ему новое имя: Codeisok. Читается «код из ок» и означает, что с кодом всё хорошо.
Выражение является внутрикорпоративным мемом и используется в диалогах:
— Сделай ревью моей задачи, пожалуйста.
— Посмотрел, codeisok’нул!
И сегодня мы рады сообщить, что Codeisok доступен для open-source-сообщества. Инструмент уже работает в нескольких проектах вне Badoo и имеет положительные отзывы (например, wetrend.com и turbocontract.ru). Мы надеемся, что вам он тоже понравится и будет полезен. Улучшения и пулл-реквесты мы будем с радостью принимать — всегда рады тем, кто помогает развивать open-source-инструменты.
Также, пользуясь случаем, хочу выразить огромную благодарность всем, кто помогал развивать и улучшать проект. Ребята, спасибо! Вы невероятно крутые!
Никиль Верма
Стас Игнатенков
Александр Измайлов
Антон Довгаль
Юрий Насретдинов
Николай Крапивный
Константин Дзюин
Милош Попович
Александр Трегер
Евгений Махров
Денис Кореневский
Павел Матолыгин
Александр Гришин
Роман Лазарев
Александр Овсянкин
In code we trust!
Илья Агеев, Badoo
Комментарии (24)
apro
10.05.2018 21:35Ревью кода всей ветки (бранчдиффа) инструмент делать не умел
А в чем смысл ревью всей ветки сразу целиком? Слишком лениво разбивать на логические
и завершенные коммиты?uyga Автор
10.05.2018 23:46+2Ну почему же, такой подход может существовать. Но если взять в уравнение скорость, необходимость аккуратного внедрения в текущий процесс, на текущих задачах, учесть размер компании и ее рост, то все становится не так очевидно. Можно заставить 250 инженеров «правильно» оформлять коммиты перед пушем, но зачем? Какая бизнесу от этого польза?
jehy
11.05.2018 00:43Фактически, вы только что спросили «А зачем пулл реквесты?».
uyga Автор
11.05.2018 00:49Не совсем. Бранчдифф это почти то же самое что пулл реквест. Но без необходимости создания отдельной сущности. И более динамичный.
Простой пример — в коде ошибка. Идеология пулл-реквестов во-первых откладывает ее обнаружение на более поздний срок. На момент, когда девелопер оформит пулл-реквест и по нему начнутся автоматические проверки (например). А во-вторых, чтобы ошибку исправить надо пулл реквест отменить и создать новый. Это лишнее время.
В случае с бранчдиффами этого ничего не нужно. Для простоты можно считать что бранчдифф это пулл-реквест, но без лишней бюрократии :)jehy
11.05.2018 00:56Хмм, забавно, но у нас в битбакете пулл реквесты работают так же, как у вас бранч диффы:
1. Автоматические проверки начинаются при пуше в ветку. Никакой новой сущности придумывать не нужно — есть просто ветка…
2. Для исправления ошибки делается пуш в ту же ветку, и пулл реквесты сами обновляются. Кстати, на гитхабе так же работает. Так что не знаю, откуда у вас взялась проблема с бюрократией и пересозданием пулл реквестов… Может, раньше так было?uyga Автор
11.05.2018 01:06Когда мы только начали делать инструмент, сторонние инструменты были другие, да. И с тех пор немало эволюционировали.
А битбакет разве поддерживает self-hosted установку? Гитхаб может, но стоит очень уж дорого.e_Hector
11.05.2018 07:27Конечно, ведь он прямо так и называется Bitbucket Server https://www.atlassian.com/software/bitbucket/server
uyga Автор
11.05.2018 09:50Ах да, атлассиан. Уже вижу с каким скрипом это будет у нас работать. И лицензия на 500 пользователей под 20к стоит.
За ссылку спасибо.jehy
11.05.2018 11:36Да, атласиановские продукты не быстрые, тем более self hosted. Хотя джиру на 200 пользователей мне удавалось разогнать до довольно шустрой работы.
А как насчёт gitlab community edition? Бесплатно, и я его использовал в двух компаниях без всяких проблем. Про то, как там работают пулл реквесты — честно говоря, не помню, но всегда можно допилить.uyga Автор
11.05.2018 13:16gitlab, как я уже упоминал выше, мне видится наиболее перспективным сейчас. Ребята большие молодцы, хорошо развивают свой проект.
В то же время, тогда когда мы его пробовали ранее, продукт был сильно сырым.
mirrr
11.05.2018 22:53В gogs и гитлабе реквесты включают все пуши после создания реквеста. Пересоздавать ничего не нужно. А проверки и сборки работают по пушу в ветку, зачем ждать реквеста?
jehy
11.05.2018 00:49Глянул по диагонали — вроде нет отличий от того, как это в битбакета и гитлабе сейчас. Может, не заметил что-то? Было бы хорошо, кроме исторического обзора (который очень интересный, спасибо), ещё с современными аналогами сравнить.
uyga Автор
11.05.2018 13:33Да, видимо было бы неплохо сделать обзорную статью со сравнением :) Давайте так: я дам вводную, а после попробую найти время на полный обзор, но не обещаю что время найдется. Это же кучу инсталляций и замеров надо делать опять, причем держа в голове что это для публичной статьи :)
Вводная:
1) Инструмент должен быть self-hosted.
2) Инструмент должен иметь возможность доработки по мере появления запросов на новые фичи.
3) Бесплатный опенсорсный лучше платного проприетарного :)
4) Инструмент должен поддерживать разделение доступа по репозиториям.
5) Общее кол-во репозиториев в системе — чуть менее 250.
6) Самый большой репозиторий содержит ~130 тысяч файлов, весит около 5 гигабайт. История коммитов в нем на текущий момент ~ 670 тысяч коммитов.
Большинство инструментов, которые мы пробовали, начинали фейлить на 6м пункте — загружали историю нашего репозитория по нескольку дней, недель и т.д. И потом безумно тупили. codeisok же не делает никаких внутренних индексов и работает поверх гита. Получается быстро.
kemsky
11.05.2018 01:57А чем Upsource не понравился?
uyga Автор
11.05.2018 13:39Вот тут в комментариях выше, я описал вводную. Эксперименты с Upsource у нас закончились тем, что в течении двух недель инструмент не смог индексировать историю самого большого репозитория. Мы писали в Jet Brains (у нас есть там контакты со времен укрощения teamcity). Нам ничего вразумительного не смогли посоветовать, а только попросили предоставить им этот самый репозиторий, чтобы они на живых данных проверили что там так долго индексируется. Этого мы, разумеется, себе позволить не смогли.
kemsky
11.05.2018 16:25У нас комитов поменьше, но тоже индексировал не быстро, кроме этого занимал очень много места, в остальном понравился.
host13
12.05.2018 22:56Я правильно понял, что вы именно идеологию фичебранчей ввели, а не Git? Т.е. Git внедрили как довесок? Не совсем очевидны преимущества Git перед SVN в плане фичебранчей.
uyga Автор
12.05.2018 22:58Правильно, в первую очередь нам нужно было разделить задачи по веткам. Один из инструментов, который хорошо и удобно это позволяет делать — git.
JediPhilosopher
А мне вот интересно, насколько вообще ревью кода полезная вещь?
Я работал в двух конторах, где оно было внедрено на обязательном уровне, плюс еще в паре мест его пытались внедрить, но там оно быстро глохло.
В первой я работал еще джуниором, и вот там оно мне было реально ползено — фидбек от опытных спецов помогал мне расти + защищал кодовую базу от моего неумелого говнокода.
Во второй, много лет спустя, в команде были уже опытные спецы, и вот тут-то от него было больше вреда чем пользы. За год работы там я в чужом коде смог на ревью найти одну багу-опечатку при копипасте (что наверное проще выявить хорошим статическим анализом кода) и один раз предложил некое архитектурное исправление. Примерно то же самое было и в отношение моего кода. Т.е. какой-то реальной пользы от него было мало, все и так писали неплохой код. При этом в тривиальном коде обычно нет ошибок, а нетривиальный слишком сложен чтобы его сходу прочитать, понять и что-то там найти, в итоге на ревью быстренько формально проглядывали по верхам, не вдаваясь в подробности т.к. и так работы много.
Зато постоянно попадались затыки: человек (особенно если это тимлид) оказывается слишком занят и куча пулл-реквестов висит у него на ревью долгое время, не давая вмерджить их и выкатить фичу, тормозя весь процесс.
В общем я на это все посмотрел, и там где я сам являюсь главным ревью внедряю только для джуниоров, для всех остальных по желанию.
jehy
Это у вас какой-то странный случай. У нас на ревью непрерывно что-то интересное находится. Причём количество вопросов по ревью никак не соотносится с уровнем разработчика. Сениор это тоже человек, который может не выспаться, просмотреть описку, забыть поправить копи пейст, написать в горячке неоптимальный код, забыть про то, что происходит в параллельных задачах, и так далее. А цена его ошибки обычно выше, чем у джуниора.
К слову, одна из вещей, которые я, как техлид, не пропускаю на ревью — это «нетривиальный код». Если мне надо думать, что и как делает какой-то код, то там почти наверняка что-то не так.
JediPhilosopher
Вот никогда не понимал такого подхода, так же как и смежного понятия «самодокументирующийся код». В какой-нибудь компании-интеграторе такой подход оправдан, так как там практически весь код — это перекладывание объектов из одного апи в другое, из одной коллекции в другую, и что-то нетривиальное действительно встретить сложно. Сложные решения там могут быть на уровне архитектуры, но каждый конкретный кусок кода довольно прост. Но и в таком случае код-ревью может быть затруднено, например когда человек выкатил какое-то крупное изменение, над которым работал последние пару недель — десятки файлов, сотни правок в разных местах, его даже прочитать по верхам сложно и долго, а уж понять и осознать, особенно не владея контекстом задачи которая делалась — слишком много времени тратить.
Ну и не везде код исчерпывается простыми операциями. Сложным может быть код, который считает какие-нибудь хитрые математические вычисления (причем хитрыми могут быть как сами формулы, так и тот извилистый путь которым к ним пришли), или реализует какой-нибудь небанальный алгоритм о котором человек раньше не слышал (вот мне тут понадобилось написать определение зоны видимости ребра полигона на плоскости, если видимость точки сделать тривиально, то видимость ребер уже нет, надо статьи на эту тему почитать). Но его тоже приходится ревьювить. И либо ревьювер тратит кучу времени на вникание в тему (вместо выполнения своих прочих обязанностей), либо ревью оказывается чисто формальным.
jehy
Крупных изменений нужно стараться избегать. Или, по крайней мере, разносить изменения по разным коммитам, и комментировать в пулл реквесте, на что нужно или нет обращать внимание (регулярно так делаю со своими пулл реквестами). Ревью одной задачи у меня обычно занимает от 5 до 20 минут, обычно делаю за кофе.
А ещё ревью занимаюсь не только я, но и сениоры команды — это помогает не допускать ситуаций, когда что-то «виснет» на ревью, да и вообще не закопаться насмерть.
Насчёт глубины погружения в код на ревью — тут нужен баланс. С одной стороны, нельзя позволить себе закопаться, с другой — ревью должно приносить больше пользы, чем статический анализатор. У нас получается — часто просто засчёт того, что свежий взгляд подмечает проблемы, которых не видит автор.
ApeCoder
ИМХО, по крайней мере, надо написать так, чтобы
uyga Автор
Хороший вопрос. У меня есть отдельная статья про «правильное» ревью и «неправильное» ревью, где я описываю распространённые ошибки. Скоро опубликуем и подискутируем там в комментариях.
Спасибо!