Всем привет, меня зовут Михаил Поливаха, я являюсь техническим лидером проекта Axelix.
В Axelix со временем добавляется всё больше и больше сторонних контрибьютеров, которых мы очень приветствуем. И всё больше и больше я сталкиваюсь с некоторым непониманием некоторых строгих гайдлайнов, которые установлены на проекте. Именно сегодня, речь пойдёт про размер PR-а.
У нас есть собственный GitHub Action, который блокирует PR в том случае, если суммарное количество измененных строк кода превышает 500.
В этой статье я хочу рассказать про то, почему такое правило важно, и почему оно вам тоже нужно.
Откуда ноги растут
В современном мире AI Agent-ов количество кода растёт довольно быстро. Растёт скорость изменений.
И я считаю, что это крайне важно, чтобы скорость этих изменений не влияла негативно на качество продукта и кодовой базы. Какое-то влияние будет, но хотелось бы его ограничить.
Для этого, мы настраиваем так называемый процесс Quality Gate. В него входят тесты, linter-ы, статический анализ кода и т.д. И очень важным компонентом Quality Gate является ревью CODEOWNER-а. Мы в Axelix хоть и имеем Code Rabbit, и наши PR-ы смотрит усиленный скилами и чем только можно AI Agent в облаке, финальное решение принимает всегда человек.
Был в своё время довольно известный research (“Needle in the Repo”), в котором говорилось, что:
The main limitation exposed by NITR is not raw implementation ability alone, but difficulty maintaining design discipline during repository-level change
То есть, речь о том, что AI Agent-ы, как правило, могут сделать хорошее, качественное локальное изменение, но часто они не до конца учитывают архитектуру проекта целиком. И если дать AI Agent-у разгуляться, то система работать-то будет. Но если потом посмотреть на неё немного сверху, то всё это будет выглядеть как какая-то каша. Я думаю, многие с этим на практике сталкивались (Вы не одни такие, research говорит о том же).
Так вот, моя мысль в том, что по крайней мере в Axelix, финальное решение о том, как изменение вписывается в общую картину проекта - вот это решение крайне важное, и его всегда принимает человек.
А раз его принимает человек, и раз оно такое важное, то это значит, что ревью кода человеком должно быть качественным. Потому что если человек допустит архитектурную проблему в кодовую базу, а потом поверх неё начнет генерировать AI, то сами можете догадаться, как потом это заканчивается.
В общем, задача - сделать так, чтобы ревью человека было максимально качественным. Настолько, насколько возможно.
Размер PR-а
И вот тут мы подходим к основной теме. Последние крупные research-и, проводимые в Microsoft, Google и других гигантах - все говорят об одном: Самый разрушительный эффект на процессе качества ревью PR-ов оказывает размер вносимых изменений.
Иными словами, чем больше PR - тем хуже будет качество ревью, то есть человек не выполнит свою роль в Quality gate. При этом качество не просто хуже, а если постараться построить график количества дефектов/багов/проблем, которые были внесены в рамках PR-а к размеру того самого PR-а, то будет очевидно, что пересекая границу где-то около 400 строк кода, качество изменений падает нелинейно.
Почему же так происходит?
Люди, конечно, разные, и у всех attention span разный и разная короткая память. Но даже стойкие люди начинают терять полный контекст изменения, переходя границу в 400 измененных строк кода в одном патче. И на практике человеку становится сложно осуществить его главную функцию как human reviewer-а в эру AI - понять, как изменение вписывается в общую картину.
Именно поэтому, в Axelix, мы со временем установили жёсткое правило в CI на размер PR-ов. Любой PR, который размером превосходит 500 строк кода (изменённых, удалённых и добавленных) просто валит билд.
Вводить такую блокирующую проверку лучше поэтапно, потому что люди, как правило, бывают к этому не сразу готовы. Об этом будет чуть ниже.
Частые Возражения
При попытке ввести такую практику, вы, скорее всего, столкнетесь с рядом возражений от коллег и людей вокруг. Это нормально. Ниже я опишу, как на нашем опыте люди реагируют на такие ограничения.
Возражение 1. А что если моя задача по природе очень большая?
Поначалу всегда будет казаться, что моя задача слишком большая, и что я не могу её разбить на небольшое количество мелких PR-ов. На практике это совсем не так.
Вот мы в Axelix живём с этим ограничением уже довольно длительное время. Из наблюдений я могу сказать, что когда отсутствует явное, блокирующее требование делать PR-ы маленькими - люди просто не думают над тем, как структурировать внедрение своей фичи в проект. Просто хлопнуть одним PR-ом и всё!
Вот давайте честно, мы же все знаем, что если в логах сборки WARNING, а не ERROR, который валит билд, то всем до лампочки. А вот уж если ERROR и сборка не проходит, ну вот тогда фиксить мы будем.
И тут то же самое. Поэтому важно в какой-то момент иметь проверку на этапе CI. Пускай сначала она будет принимать PR-ы до 1000 строк кода. Со временем вы эту границу будете сдвигать ближе в сторону 400-500. Но имея эту проверку, вы будете вынуждать человека думать над вопросом: “А как же мне разбить задачу таким образом, чтобы её можно было внести 3-4 самодостаточными PR-ами?” Почти всегда такое сделать можно, это не rocket science.
Подобно тому, как ERROR лог и падающая сборка вынуждает вас исправлять проблему, также и проверка на этапе CI - она вынуждает разработчика думать над структурированием своих изменений.
По опыту Axelix говорю - даже самые казалось бы крупные задачи можно декомпозировать на самодостаточные небольшие изменения. Нужен лишь хороший механизм enforcement-а. Со временем, ваша команда начнёт думать над структурой и декомпозицией перед тем, как начнёт реализацию. И это хорошо.
Возражение 2. Но ведь тогда упадёт скорость внесения изменений!
Я слышал это много раз, но это очень спорное заявление. Есть большое количество research-ей, вот кстати недавный на примере GitHub. Почти все они сходятся в ответе: в большинстве своём никакой значимой корреляции между размером PR-а и скоростью разработки нет.
Если почитать research-и, то чаще всего (предположительно) это происходит по двум вещам.
Если PR большой, то ревьюверы смотрят его сквозь пальцы и апрувят довольно быстро. Казалось бы, вот она скорость.
Но не забывайте, что большие PR-ы более вероятно приведут к проблемам и дефектам в коде. Вам/AI Agent-у придётся тратить время на их исправление. Кроме того, нужно будет тратить время на структурный рефакторинг (ведь архитектурный техдолг внести с большим PR-ом проще).
И эти две вещи, как правило, друг друга компенсируют. Но это лишь догадка о механизме - так это или нет, точную причину данные не показывают. Видно лишь итог: значимой корреляции между размером PR и скоростью разработки нет.
Угловые случаи
Конечно, на практике, при адоптации такого механизма, не всё так просто. Есть некоторые угловые кейсы.
Пример: Lock файлы
UI часть Axelix написана на React-е. То есть по сути, UI это просто nodejs проект с package.json/package-lock.json. Очевидно, что добавление зависимости в package.json, или смена требуемой версии nodejs в .nvmrc и т.п. - всё это способно довольно сильно изменить package-lock.json, потенциально на многие сотни строк кода. Как быть с этим?
Или другой пример. Мы используем AI Agent-ов довольно активно в разработке, и у нас используется инструмент, под названием APM (Agents Package Manager). В нём мы объявляем весь harness, который нужен для изменения кода в Axelix с помощью AI Agent-ов.
У APM есть основной файл - apm.yml, в нём объявляются все “зависимости” (skill-ы, mcp сервера и т.д.) для AI Agent-ов. А есть другой файл, apm.lock.yaml, который семантически похож на package-lock.json - он фиксирует конкретные версии установленных зависимостей, во многом, с целью безопасности.
И этот файл тоже автоматически генерируется и в крупных monorepo может быть довольно большой.
Другой пример: License Headers
В community программистов довольно давно была дискуссия на тему того, а нужны ли License Header-ы вообще. Есть некоторое мнение в западном правовом сегменте, что наличие LICENSE файла в дистрибутиве (в Jar файле, например) - этого уже вполне достаточно. Но на практике всё сложнее, и, например Axelix добавляет License Header-ы во все source code файлы проекта. У нас на это даже настроен линтер.
В прошлом я как-то писал статью про лицензирование и его детали. Я думаю, скоро я выпущу продолжение этой статьи (возможно, не одно), где расскажу, в чём сыр-бор с License Header-ами.
А пока что надо признать - вот добавил человек 8-10 небольших source файлов в PR-е, и это уже + 150-170 строк кода только от License Header-ов. С этим что делать?
Как это делаем мы
Давайте суммируем.
Проблема в том, что есть конкретные файлы, трогая которые мы рискуем легко перевалить за границу 500 LoC в PR-e. Кроме того, подавляющее число наших source файлов содержит License Header-ы. С ними что делать?
Тут надо сделать пару шагов назад и вспомнить, что проблема больших PR-ов в том, что они кратно увеличивают когнитивную нагрузку на ревьювера (LLM-у тоже большие PR-ы, кстати, не нравятся, но это другая тема), и как следствие, становится на порядок проще пропустить крупную проблему.
Добавляют ли License Header-ы, lock и т.п. файлы дополнительную когнитивную нагрузку во время ревью? Наш ответ: На данный момент, мы думаем, что нет, не добавляет.
И поэтому, мы решаем проблему примерно так:
Наш собственный action игнорирует автогенерируемые файлы при подсчёте количества строк в PR-е. А чтобы учитывать License Header-ы, например, мы как раз и имеем верхнюю границу размера PR-а в 500 строк кода, а не в 400. Выковыривать License Header-ы из подсчёта на практике будет не так просто, поэтому, проще просто сделать estimate того, насколько много их будет. Возможно, estimate неверный, и мы будем его менять, но сейчас это выглядит так.
Вывод
Делайте маленькие PR-ы. В мире, когда изменения активно генерирует AI и скорость изменений растёт, особенно важно иметь строгий quality gate, важной частью которого является review человеком.
Современные ресерчи показывают, что чтобы ревью было качественным, количество строк кода в PR-е не должно превышать 400 строк. Прямо настройте это правило в CI. Если оставить это просто гайдлайном - работать не будет. Чтобы дать команде время адоптироваться, можете начать не с 400-500 строк кода - можно сначала взять планку повыше, а далее, со временем, её снижать.
Всем успехов!