Workflow, PR, code review

Что отвечать

Atomic commit - один коммит = одно логически законченное изменение, которое можно описать одним предложением и которое **компилируется и проходит тесты само по себе**. Не «WIP», не «fix typo + add feature + reformat». Зачем: `git bisect` работает только если каждый коммит собирается; revert одного логического изменения становится одной командой; review понимает «что именно сделал этот коммит».

Что хотят услышать

Senior должен: - назвать три критерия: одно изменение, описывается одной фразой, собирается+проходит тесты на своём моменте - объяснить связь с bisect: если коммиты «сборные» - бизект найдёт коммит, но не поймёт что внутри него виновато - сказать что atomic не значит «маленький»: коммит на 500 строк может быть атомарным (миграция БД целиком), а на 5 строк - нет (fix typo + add unused import) - назвать инструменты дисциплины: `git add -p` для добавления кусками, `git commit --fixup=<sha>` для прицельной правки, interactive rebase для разбиения большого коммита на части - упомянуть что в командах с squash-merge атомарность теряется на main, но всё ещё важна для review до merge

Подводные камни

• ✗ Сказать «atomic это маленький коммит» - размер не критерий, критерий это «одно логическое изменение»
• ✗ Делать коммит «refactor + new feature» - revert потом потребует переписывать руками
• ✗ Скипать `git add -p` ради скорости и засовывать в один коммит правки нескольких несвязанных файлов

Follow-up

• ? Как разбить большой коммит на серию атомарных через interactive rebase?
• ? Чем `git add -p` помогает делать атомарные коммиты?
• ? Что произойдёт с `git bisect` на репозитории с broken-коммитами?

Что отвечать

Формат `<type>(<scope>): <subject>` где type из фиксированного списка: feat, fix, docs, refactor, test, chore, perf, ci, build. Даёт три вещи: автоматический changelog (`changelogen`, `semantic-release`, `release-please` парсят commits и формируют release notes), автоматический semver bump (feat = minor, fix = patch, `BREAKING CHANGE:` в теле = major), и однородный язык в истории - reviewer сразу видит тип изменения.

Что хотят услышать

Senior должен: - назвать связку с semver: feat → minor, fix → patch, BREAKING CHANGE → major, и инструменты которые это автоматизируют - сказать что conventional commits работают и без автоматики - просто как дисциплина чтения истории - упомянуть commitlint + husky как способ enforce формат на локальном pre-commit hook, или GitHub Action на PR-title - назвать что при squash-merge формат должен быть в PR-title (это и попадёт в main как commit message), а не в отдельных коммитах PR - предупредить: в командах без релизной автоматики conventional commits часто превращаются в карго-культ без выгоды

Подводные камни

• ✗ Использовать conventional commits как «правило ради правила» без changelog/semver автоматики - тогда это просто формальность
• ✗ Писать `chore: stuff` без scope и пояснения - формат соблюдён, смысл нулевой
• ✗ Не enforce'ить формат в CI - команда быстро разбредётся между «feat:» и «Feature:»

Follow-up

• ? Как настроить commitlint + husky чтобы блокировать неправильный формат?
• ? Что в release notes у `semantic-release` если в PR не было ни feat ни fix?
• ? Где писать `BREAKING CHANGE:` - в subject или в body коммита?

Что отвечать

Хороший PR: одна задача, понятный title и описание (что/зачем/как тестировал/breaking changes), reasonable размер (≤ 400 строк обычно), атомарные коммиты, проходящий CI, явно отмеченные «обсудим» места в собственных review-комментариях. Плохой PR: «refactor + feature + dependency bump», 3000 строк без описания, reviewer'у предлагается «разобраться по diff'у», красный CI с комментарием «потом починю».

Что хотят услышать

Senior должен: - назвать checklist хорошего PR-описания: контекст, что сделано, как тестировал, что **не** сделано/откладывается, скриншоты для UI - сказать про reasonable size: исследования (Google, SmartBear) показывают что defect detection падает после 400 строк, у автора падает дисциплина после 800 - упомянуть «отметь спорные места сам» - саморевью с комментариями экономит reviewer'у час - назвать практику draft PR / WIP PR для early-feedback без требования полного review - сказать что для большой задачи лучше серия PR (один логический кусок = один PR), чем один мега-PR

Подводные камни

• ✗ Закидывать огромный refactor + feature одним PR - reviewer либо застрянет на неделю, либо мерджнёт не глядя
• ✗ Писать описание из одной строки `fix bug` - reviewer не понимает что именно ты починил
• ✗ Спрятать «попутный refactor» в feature-PR - размывает review-фокус

Follow-up

• ? Когда уместен draft PR vs ready-for-review?
• ? Что должно быть в PR-template чтобы автор не забыл важное?
• ? Как разбить готовый огромный PR на серию без потери истории?

Что отвечать

Сначала прочитай PR-описание и контекст, потом смотри код. Различай три типа комментариев: **must-fix** (баг, security, нарушение контракта), **should-fix** (плохая читаемость, потенциальная проблема), **nit/style** (стиль, вкус). Помечай тип явно, иначе автор тратит силы на все одинаково. Не блокируй PR на nit'ах. Если 5+ замечаний - синхронный разговор быстрее асинхронной переписки.

Что хотят услышать

Senior должен: - назвать конвенцию префиксов комментариев: `nit:`, `suggestion:`, `question:`, `blocker:`, чтобы автор сразу видел приоритет - сказать что reviewer обязан назвать **что одобряет**, не только что критикует - иначе автор думает что весь PR плохой - упомянуть Conventional Comments (`praise:`, `nitpick:`, `suggestion:`, `issue:`, `question:`, `thought:`, `chore:`) как готовый словарь - назвать «smell-метрики»: PR висит > 2 дней - надо ping или реассайнить; > 10 round-trip'ов комментариев - надо созвон; > 1 reviewer молчит неделю - reassign - сказать что review должен фокусироваться на **смысле**, синтаксис и форматирование закрываются линтером, не reviewer'ом

Подводные камни

• ✗ Блокировать approve из-за `nit:` - автор переделывает день, ценности ноль
• ✗ Молчать про хорошее - автор не понимает что было удачно сделано
• ✗ Спорить про tabs vs spaces в review - это работа prettier/black, не reviewer'а

Follow-up

• ? Чем Conventional Comments отличаются от обычных review-replies?
• ? Когда лучше synchronous code review vs async?
• ? Как настроить required reviewers в GitHub чтобы review не блокировал на nit-ах?

Что отвечать

`.github/CODEOWNERS` назначает автоматически: «эти файлы трогают эти люди/команды». При открытии PR с правкой `infra/**` Github сам реквестит review у `@org/sre`. Branch protection на main: требует N approve'ов, статус checks (CI зелёный), часто required-review-from-codeowners. Force-push и delete запрещены. Вместе это даёт: «нельзя замержить в main без зелёного CI и одобрения нужных людей».

Что хотят услышать

Senior должен: - назвать формат CODEOWNERS (glob + handles), сказать что порядок строк значим (последняя совпавшая выигрывает) и что teams требуют write-доступ к репо - объяснить required status checks: список workflows которые должны пройти (build, test, lint, security-scan) - упомянуть «require linear history» как опцию которая запрещает merge-коммиты, разрешая только rebase/squash - назвать `restrict who can push to matching branches` для запрета прямых push'ей в main, всё только через PR - сказать что admin-bypass branch protection обычно открыт для emergency-фиксов, но это должно логироваться и обсуждаться

Подводные камни

• ✗ Думать что CODEOWNERS блокирует merge - не блокирует сам по себе, блокирует только если в branch protection включено `require review from code owners`
• ✗ Положить CODEOWNERS в неправильное место - валидно в `.github/`, `docs/`, или корне, но не где попало
• ✗ Защищать только main, забыть про release/* и develop - тогда обход через них

Follow-up

• ? Что произойдёт если человек из CODEOWNERS уволился но GitHub-аккаунт остался?
• ? Чем `require linear history` отличается от запрета merge-strategies?
• ? Как настроить branch protection через `gh api` для нескольких репо разом?

Что отвечать

Semver (MAJOR.MINOR.PATCH): breaking-изменение → MAJOR++, backward-совместимая фича → MINOR++, патч-фикс → PATCH++. Инструменты типа `semantic-release`, `release-please`, `changesets` читают коммиты от прошлого тега (обычно через Conventional Commits), определяют bump-тип, генерят `CHANGELOG.md`, создают git tag и GitHub release. Дисциплина в формате коммитов даёт «нажатие одной кнопки = новый релиз с релиз-нотами».

Что хотят услышать

Senior должен: - связать три части: Conventional Commits → парсер → semver bump + changelog + tag, и сказать что без первого автоматизация невозможна - назвать инструменты по эко-системам: `semantic-release` (Node), `release-please` (multi-language Google), `changesets` (Node, monorepo-first), `python-semantic-release` (Python) - упомянуть что `0.x.y` имеет особые правила: minor может быть breaking, проект ещё в pre-release - сказать что для библиотек breaking changes особенно дороги - прыжок мажорки заставляет потребителей мигрировать, поэтому часто запоздалые мажорки накапливают breaking - назвать `git tag -s v1.2.3` для GPG-подписанных релиз-тегов как часть supply-chain security

Подводные камни

• ✗ Бампать MAJOR на каждую мелкую API-правку - потребители устанут от миграций
• ✗ Думать что `0.x.y` это «такой же semver» - в 0.x minor может быть breaking, semver это явно разрешает
• ✗ Сделать `git tag` без push - тег остаётся локально, CI про релиз не знает

Follow-up

• ? Чем `release-please` отличается от `semantic-release`?
• ? Что делать с changelog если до сих пор писали коммиты в свободной форме?
• ? Как подписать тег GPG-ключом и почему это важно для релизов?