2019-09-13

Q&A Обязательные код-ревью

Привет. У меня в команде коллега (нас всего двое), который не кидает на ревью какие-то мелкие PR в 1-5 строк, а сразу мёржит в мастер. Объясняет это тем, что это мелкие незначительные изменения, и на ревью не кидает их если уверен, что там всё норм. Таких мёржей без ревью может набраться чуть ли десяток за неделю. Я с этим не согласен и периодически бомблю в тихую надумывая пойти и попытаться окончательно решить ситуацию в пользу обязательных ревью, пусть даже мелких. Сейчас подумал, мб всё норм и не стоит бомбить по этому поводу? Что посоветуешь?

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

Но скорее всего у вас не такая команда и не такие подходы. Значит большинство изменений идут через процесс ревью. Я в этом вижу такие проблемы:

  • Ты упускаешь часть изменений, раз они сразу уходят в мастер, ты их попросту не видел. Зачем такие сюрпризы?
  • Появляется поле для спекуляций: что считать мелким изменением? Количество строк? или "содержание"? а где границу провести?
  • Cубъективность в оценках может вызвать ненужные обсуждения после.
  • В команде существует два параллельных процесса. Как новичку объяснить, какой процесс выбрать?
  • Если что-то пойдёт не так, то об изменении будет знать лишь твой коллега, а решать проблему будешь ты, так уж мир устроен.

Почему твой коллега так делает? Если PR "простой и там всё норм", то такие вещи на ревью должны находиться пару минут. Правда ли ему важно экономить эти минуты? Возможно ты долго реагируешь на просьбу о ревью, в таком случае можно договориться и помечать "простые" ревью как-нибудь. Чтобы они шли в начало очереди.

Я обычно стараюсь запретить коммиты в мастер вообще всем. Только через PR, CI и прочие штуки. Можно даже прекоммит хук добавить.

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