Какой правильный workflow должен быть для code review?

Question

[Александр]

Какой правильный workflow должен быть при проведении code review?
Например, у нас в git-репозитории есть ветка "dev", в которой ведется разработка, и я вижу такие варианты:
1. Задачи разрабатываются в отдельных ветках и мержатся в "dev" только пройдя code review.
2. Задачи разрабатываются в отдельных ветках и мержатся в "dev" сразу же, без ревью. Но, дополнительно создается еще одна ветка (скажем, "review"), в которую мержатся те же задачи, только после прохождения code review. Ветки "dev" и "review" периодически синхронизируются.

В варианте №1 я вижу минус в том, что очень замедляется деплой задач. Фактически, пока не освободится человек и не сделает ревью, задача может очень долгое время не попасть в ветку "dev".
В варианте №2, как мне кажется, большая вероятность конфликтов за счет того что приходится постоянно синхронизировать две ветки.

А как делаете вы?

Answer 1

[Vit]

Без ревью мержим только хот фиксы.
На остальные задачи делаем пулл реквесты, обязательно проходящие ревью.

Comments

[Александр]

Но ведь тогда запрос на ревью может провисеть несколько дней в ожидании. В то время как за эти дни изменения могли уже тестировать тестировщики.

[samizdam]

Александр: определите, какова цель ревью?

[Александр]

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

Answer 2

[leclecovich]

В команде настроили систему Code Review на запрет коммита в develop/master, только мерж из ветки. Для мержа нужен хотя бы один апрув кого-то из команды и успешный прогон юнит-тестов. Для последнего была настроена интеграция с CI.

Как результат - повысилось качество кода, тесты всегда в актуальном состоянии. Такой подход позволяет находить большое количество глупых ошибок еще на стадии ревью. Как ни странно, но времени на ревью тратится не так много - порядка 5% - 10% от общего времени выполнения задачи.

Answer 3

[samizdam]

Почему я рекомендую делать ревью до мержа (вариан 1):
1. психологически: легче вернуть задачу разработчику и внести изменения в код до того, как ветка фичи ушла в основную
2. технически: с точки зрения тестирования и конфликтов также легче внести изменения в тематическую ветку до мержа, чем после в основную
Пусть, в случае если Вы доверяете разработчикам, и не успеваете провести ревью всех пул-реквестов — дайте тогда разработчикам возможность мержить самим, а в интерфейсе (gitlab / github / bitbucket) всегда можно отревьюить и закрытый мерж-реквест, пост-фактум. Просто принять соглашение, чтобы все мержи в основную ветку осуществлялись через один этот интерфейс.