Pull request как процесс

В чистом Git нет понятия «pull request». Есть git request-pull (генерирует текстовое уведомление для maintainer'а -как раз такое, какое используется в ядре Linux), но это не то. pull-request в виде кнопки и обсуждения -изобретение GitHub 2008 года.

Что PR накладывает поверх Git:

• Аудит. Каждая ветка, перед тем как попасть в main, проходит видимый этап: кто-то прочитал код, согласился (или не согласился). История этого согласия сохраняется навсегда.
• Discussion. Контекст изменения живёт рядом с самим изменением, а не в Slack-чате или email-треде.
• Gating. Если включены branch protection rules, ничего не попадёт в main до выполнения условий (review, проверки CI).
• Видимость. Любой в команде видит, что идёт в работе.

Без PR можно жить. Большие OSS-проекты до сих пор живут через mailing list patches. Маленькие личные проекты тоже -ты сам себе автор и ревьювер. PR -это инструмент команды, не инструмент Git.

Понимать это разделение полезно, потому что многие фрустрации типа «GitHub меня тормозит» -на самом деле «процесс меня тормозит». Замени GitHub на GitLab -получишь тот же процесс, только другой UI. Уберёшь процесс -потеряешь видимость и гейтинг.

В следующих разделах разберём процесс по частям.

Стандартный цикл выглядит так:

draft -> ready for review -> reviewed -> updated -> merged

                                 \-> closed (без merge)

Draft. PR, который ещё не готов к review. Создаётся через gh pr create --draft или в UI кнопкой «Create draft pull request». Подробности про draft -в draft-pr и главе 16. Драфт сигналит коллегам «работа не закончена, не ревьювите сейчас». При этом CI на нём работает (полезно, чтобы видеть, падает тест или нет, пока пишешь код).

Ready for review. Автор нажимает «Ready for review» (или gh pr ready). PR становится обычным, можно ревьювить. Если настроены required reviewers через CODEOWNERS -они автоматически назначаются на этом этапе.

Reviewed. Ревьюверы оставили один из трёх вердиктов: Approve (одобрил), Request changes (нужны правки), Comment (нейтральный коммент без вердикта). У PR появляется состояние reviewDecision: APPROVED / CHANGES_REQUESTED / REVIEW_REQUIRED.

Updated. Автор увидел review, поправил, запушил новые коммиты. PR обновился. Если был CHANGES_REQUESTED, ревьюверы должны переоткрыть review -обычно через кнопку «Re-request review».

Merged. PR смерджен в целевую ветку. Стратегия слияния определяется настройками репо и кнопкой (merge-strategies): обычный merge commit, squash, или rebase. PR закрывается автоматически. Ветку обычно удаляют сразу (это одна из настроек репо: «Automatically delete head branches»).

Closed без merge. Автор или maintainer решил, что PR не нужен. Закрывается без merge'а. Ветка остаётся, можно переоткрыть позже.

Этот цикл и есть «процесс». Все остальные обсуждения (стиль review, размер PR, branch protection) -детали внутри него.

Самое важное про PR -размер. Маленький PR ревьювится за 10 минут. Большой PR -за два часа, и шансы, что ревьювер устанет к концу и пропустит баги, растут пропорционально.

Эмпирическое правило: до 400 изменённых строк (+/-) - зелёная зона. До 800 -жёлтая, ревьювер ещё справится, но без энтузиазма. Больше 1000 -красная: либо разбивай, либо получишь поверхностный review.

Откуда эти числа: исследования показывают, что после 400 строк качество review резко падает (Cisco исследовала ~10 000 review в начале 2000-х; цифры с тех пор не сильно изменились). 1000 строк -это уже не review, а «беглый просмотр с одобрением, потому что отказать поздно».

Способы держать PR маленьким:

• Один PR -одна тема. Если ты переименовываешь файлы и одновременно меняешь логику, разбей на два PR: rename отдельно, логика отдельно. Diff станет читаемым.
• Готовая фича через несколько PR. Большая фича -пять мелких PR (skeleton, потом наполнение, потом UI, потом тесты, потом интеграция), не один огромный.
• Не смешивать formatting и логику. Если в твоём проекте не настроен auto-format ([[pre-commit framework]]), и тебе пришлось поправить отступы в файле, который ты ещё и изменил -сначала commit format only, потом commit change logic. В PR это можно даже не разделять, но diff с separate commits читать проще.
• Не трогать unrelated файлы. «Заодно поправил соседнее» -классический способ раздуть PR. Сделай это отдельным PR.

Размер коммитов внутри PR тоже важен. Логически связные изменения -один коммит. Несвязанные -разные коммиты. Это особенно ценно для squash-merge: маленькие осмысленные коммиты с понятными сообщениями (conventional-commits) легко читать в diff и в blame.

Описание PR. Минимум:

• Что меняется (одна строка).
• Почему. Ссылка на issue или короткое описание мотивации.
• Как тестировать. Шаги, чтобы ревьювер мог проверить, что работает.
• Скриншоты или видео, если меняется UI.

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

Чтобы каждый PR в репо имел структурированное описание, GitHub поддерживает шаблон. Файл .github/PULL_REQUEST_TEMPLATE.md в корне репо подставляется в поле описания при создании любого PR. Подробности -в pr-template.

Минимальный полезный шаблон:

## What

<!-- Что меняется в одну-две строки. -->

## Why

<!-- Какую проблему решает или ссылка на issue.

     Например: Fixes #142 -->

## How to test

1. Шаг 1

2. Шаг 2

3. Ожидание

## Checklist

- [ ] Тесты добавлены или обновлены

- [ ] Документация обновлена (если нужно)

- [ ] Сам прошёл по diff и нет лишних изменений

Чек-лист -не для красоты. Когда автор отмечает галки, он реально перечитывает свой PR -и часто находит то, что забыл. В одной из команд, где я работал, добавление чек-листа снизило долю PR, требующих re-review, на треть.

Несколько шаблонов

Если в репо есть несколько типов изменений (feature, bug fix, docs), сделай .github/PULL_REQUEST_TEMPLATE/ директорию с несколькими файлами:

.github/PULL_REQUEST_TEMPLATE/

  feature.md

  bugfix.md

  docs.md

Чтобы выбрать конкретный шаблон при создании PR, добавь в URL ?template=feature.md. На практике это используют реже, чем один общий шаблон -но в больших OSS-репо встречается.

CONTRIBUTING.md рядом

Шаблон PR должен сочетаться с contributing-md -документом, который описывает «как контрибутить в этот проект». Шаблон - это минимум, который PR должен заполнить. CONTRIBUTING -это контекст: какие тесты обязательны, какой стиль кода, какой DCO sign-off, как пинговать maintainer'ов. Один файл уточняет другой.

Что ревьювер должен делать:

• Понимать, что делает PR. Если непонятно после чтения diff и описания -попроси автора прояснить, не угадывай.
• Думать о подходе. Решает ли этот PR заявленную проблему? Есть ли способ проще? Что упущено? Не «правильно ли написан код», а «нужен ли вообще такой код».
• Проверять граничные случаи. Что если входной массив пустой? Что если файл не существует? Что если две операции выполняются одновременно? Эти вопросы лучше задать в review, чем пропустить в продакшен.
• Смотреть тесты. Покрывают ли они изменение? Тестируют реальное поведение или просто факт того, что код выполнился? Тесты с assert True не считаются.
• Думать о читателе. Через год новый разработчик откроет этот код -сможет ли он понять, почему он такой? Если нет - попроси комментарий или рефактор для ясности.

Что ревьювер не должен делать:

• Ловить опечатки и стиль. Это работа linter'а и formatter'а ([[pre-commit framework]]). Ревьювер не должен тратить время на «здесь два пробела вместо четырёх».
• Запускать тесты руками. Это работа CI. Если CI зелёный, тесты прошли. Если красный -автор должен починить до review.
• Переписывать PR за автора. Если есть существенные замечания -написать их, и автор сам внесёт. Кодить за автора в чужом PR -плохой паттерн (исключение: одна-две строки через suggestion-блок).
• Блокировать на основе вкуса. «Я бы написал по-другому» -не повод request changes. Повод -если другой подход объективно лучше (читаемость, производительность, безопасность).

Подробности про процесс code review -в code-review.

Сколько времени на review

Эмпирически: 30-60 минут на 400 строк -это нормально. Можно быстрее, если diff простой (refactor, doc). Можно дольше, если сложная логика или нужно поднять, чтобы протестировать.

Если PR требует 2+ часа review -это плохой PR (см. 17.3), а не плохой ревьювер. Скажи автору разбить.

Тон важнее содержания. Code review -это разговор между коллегами, не суд. Несколько правил из практики:

Используй suggestion-блок для тривиальных правок. На любой строке в diff'е можно начать комментарий с ```suggestion. Это создаёт inline-предложение, которое автор может применить одним кликом:

```suggestion

def parse_url(url: str) -> ParsedUrl:

```

Удобно для переименований, исправления типов, мелких поправок синтаксиса. Автор не вручную копирует -принимает изменение.

Различай «надо» и «было бы хорошо». Используй префиксы:

• nit: -nitpick, мелочь, можно не править. «nit: лишний перевод строки».
• suggestion: -предложение, обсуждаемо. «suggestion: вынести в отдельную функцию для тестируемости».
• question: -не понимаю, объясни. «question: зачем здесь retry если выше уже есть try/except?»
• blocker: или must-fix: -не сольём без правки. «blocker: здесь race condition при двух одновременных вызовах».

Это помогает автору приоритизировать. На 20 комментариев, где 18 nit: и 2 blocker: -ясно, что чинить в первую очередь.

Объясняй «почему», а не только «что». Плохо: «лучше использовать map». Хорошо: «лучше использовать map - понятнее, что мы преобразуем коллекцию, не делаем side-effect». Автор учится, не просто исправляет.

Не дублируй CI. Если linter уже пожаловался -не повторяй то же в review. У автора уже есть feedback, не добавляй шум.

Three states of review:

• Approve -код можно мерджить. Не обязательно «идеальный», достаточно «достаточно хороший». Approve с nit'ами -норма.
• Request changes -нужны правки перед merge. Используй умеренно: если есть блокирующие замечания. Если замечания мелкие -approve с комментариями.
• Comment -нейтральный отзыв без вердикта. Полезно, когда у тебя есть мнения, но решение -за владельцем code area или другим ревьювером.

Если PR на review больше суток без движения -напиши автору в Slack или другой канал. Молча сидеть на PR -недружественно.

branch-protection -набор правил, которые GitHub применяет к попыткам изменить защищённую ветку (обычно main). Без protection любой с write-доступом может запушить прямо в main, обойдя PR-процесс. С protection это становится физически невозможно.

Настройка: Settings -> Branches -> Branch protection rules -> Add rule. Указываешь паттерн ветки (main, release/*, *-prod) и опции.

Самые полезные опции

Require a pull request before merging. Запрещает прямой push в ветку. Можно только через merge PR. Базовая защита, должна стоять везде.

Require approvals (N). Минимум N approve'ов перед merge. Обычно 1, на критичных репо 2. Не ставь 0 -без approve кто угодно может смерджить свой PR.

Dismiss stale pull request approvals when new commits are pushed. После пуша новых коммитов approve сбрасывается. Без этой галки можно получить approve на одну версию, потом «втихую» добавить ещё коммит, и смерджить с уже устаревшим approve. Включай.

Require review from Code Owners. Если есть CODEOWNERS, требует approve от code owner'а конкретной изменённой части кода. См. секцию 17.8.

Require status checks to pass before merging. Список checks (обычно CI), которые должны быть зелёными перед merge. Выбираешь конкретные jobs из последних запусков.

Require branches to be up to date before merging. Перед merge'ом ветка должна содержать последний main. Заставляет автора сделать git fetch && git rebase main (или merge) до merge'а. Полезно, чтобы CI запускался на финальном состоянии кода, а не на «как было неделю назад».

Require conversation resolution before merging. Все conversation в review должны быть resolved. Не даёт смерджить PR с висящими «надо обсудить» комментариями.

Require signed commits. Все коммиты должны быть подписаны (gpg-signing). Защита от подделки авторства. На большинстве проектов опционально, на security-чувствительных -обязательно.

Require linear history. Запрещает merge-коммиты, только squash или rebase. История на main линейная. Это вопрос команды -кому-то нравится, кому-то нет.

Do not allow bypassing the above settings. По умолчанию админы могут обойти правила. Эта галка отключает обход. Включай -иначе protection превращается в «мягкую рекомендацию».

Restrict who can push to matching branches. Список людей и teams, которым разрешён push (через PR-merge). По умолчанию все с write-доступом.

Allow force pushes / Allow deletions. По умолчанию выключены, не включай. Force push в main стирает историю (force-push). Deletion удаляет всю ветку.

Что блокирует и что нет

Без bypass'а защищённая ветка не принимает:

• Прямой push (git push origin main).
• Force push (git push --force origin main).
• Delete (git push origin --delete main).
• Merge PR, не удовлетворяющий правилам.

То есть всё, что сильнее «open PR, get reviews, merge через кнопку», просто не работает. Это и есть смысл protection.

CODEOWNERS -файл в репо, который привязывает пути к командам или людям. Когда PR трогает файл по этому пути, указанные владельцы автоматически назначаются ревьюверами. Подробности -в codeowners.

Расположение: один из трёх путей:

• .github/CODEOWNERS
• CODEOWNERS (в корне)
• docs/CODEOWNERS

Один из этих, GitHub найдёт.

Синтаксис -как .gitignore, но вместо игнорирования - привязка к owner'у:

# глобальный fallback - kto vladelets всего, что не покрыто специальнее

*           @acme/leads

# backend

/backend/   @acme/backend

/backend/db/  @acme/database

# frontend

/frontend/  @acme/frontend

# документация

*.md        @acme/docs

# один конкретный файл

/SECURITY.md  @alice @bob

Правила работают по принципу «последнее совпадение побеждает». Если PR трогает /backend/db/migrations/0042.py, owner'ом будет @acme/database, не @acme/backend -потому что /backend/db/ -более специфичный путь и определён позже.

В связке с branch protection

CODEOWNERS сам по себе только назначает ревьюверов. Не блокирует merge без их approve. Чтобы заблокировать, нужно в branch protection включить «Require review from Code Owners». Тогда:

1. PR открывается, трогает /backend/db/.
2. CODEOWNERS автоматически назначает @acme/database.
3. PR может быть смерджен только после approve от кого-то из @acme/database (плюс другие правила branch protection).

Это сильная гарантия: ты не можешь случайно протащить изменение чужой подсистемы без её владельцев.

Подводный камень: team membership

CODEOWNERS-команды должны быть видны репо, иначе owner не назначится. В Settings -> Manage access -> Add team -> teams, которые ты упомянул в CODEOWNERS. Без этого GitHub считает, что team не имеет доступа к репо, и пропускает её при назначении.

На больших org это типовая болевая точка: «почему ребята из database не получили уведомление?» -потому что team была добавлена в CODEOWNERS, но не получила access к репо.

Чего CODEOWNERS не делает

• Не назначает права. Owner всё ещё нужно иметь write-доступ через прямой grant или team-membership.
• Не определяет, кто автор кода. Это про blame (blame), не про CODEOWNERS. Owner -это «кто отвечает сейчас», не «кто написал».
• Не работает на private gist'ах. Только на репо.