Retro

Cross-PR анализ — повторяющиеся паттерны

Цель: выделить системные проблемы, проходящие через несколько PR. На них надо реагировать процессом, не разбираться по каждому PR отдельно.

Важная оговорка по PR #842 (YouTube schema): scope-владелец = Olexandr (Sasha) Fedorenko (отдельный Contentful App, который пишет youtubeUploadDate в Contentful). Иван в PR #842 делал только фронт: читал поле через Delivery API и рендерил VideoObject schema. Поэтому в матрице ниже для строк, относящихся к scope Contentful App (миграция existing entries, выбор trigger, pre-merge tasks для deploy App), корневая ответственность на Саше, не на Иване. В сводках по паттернам это явно помечено.

Сводная матрица — претензии по PR

Категория проблемыPR #837 (FAQ)PR #842 (YouTube)PR #832 (pages)PR #846 (header)PR #847 (FAQ hotfix)
Невыполнение acceptance criteria из приложенной спеки✓ конфигурация поля—✓ Minisite Right Nav пропущен✓ text-only logo, search styling, dropdown center✓ bottomComponentsCollection пропущен
Делает то, чего нет в спеке———✓ dropdown center-justified—
Невалидный schema.org объект—✓ VideoObject без uploadDate (Иван — фронт-guard)✓ VideoObject без uploadDate (повтор у Ивана)——
Pre-merge / post-deployment tasks отсутствуют в PR—✓ build у ревьюера сломался (Саша — архитектура deploy App; Иван — PR description)✓ Alexis 23.02 прямо спрашивал——
Debug-код / dead code / лишние whitespace—✓ console.log✓ whitespace-only changes—✓ dead env-var fallback
Незнание / неиспользование существующих абстракций——✓ getExternalPath, Image Wrapper.credit——
Функциональный баг на проде/preview——✓ undefined/profile/... URL——
Регрессия от собственных изменений———✓ меню в 2 строки 1024-1091px—
Self-QA / responsive / accessibility пропуски—✓ ~700 unsynced (это scope Саши — Contentful App не предусматривал backfill; не Иван)✓ многочисленные✓ dark mode WCAG, RWD✓ existing data
Contentful environment ошибки✓ master vs develop————
Неполный фикс после ревью (reopen)——✓✓ sticky после фикса—
Workflow violation (commit-after-approve)————✓ TODO "when approve PR"
Squashing/rebase на расшаренной веткевозможно (TBD по git log на встрече)———TBD по git log на встрече

Топ-5 системных паттернов

1. Невыполнение acceptance criteria приложенной спеки (4 из 5 PR)

  • PR #837 — спека ICUS-214 в Jira за 2 дня до PR; реализация не по спеке.
  • PR #832 — пропущен Minisite Right Nav из spec docs.
  • PR #846 — text-only logo, search styling — прямо в мокапе v3, не реализовано.
  • PR #847 — bottomComponentsCollection — Иван сам в description обещал покрыть Right Nav и Program Detail, в коде gap.

Корневая причина: спека читается поверхностно или не сверяется с реализацией построчно перед открытием PR.

Что лечит:

  • Spec-read checkpoint в начале задачи (Иван явно подтверждает прочтение parent + attachments).
  • Side-by-side сверка мокап ↔ preview перед открытием PR.

2. Самостоятельные решения в обход спеки (positive AND negative) (2 из 5)

  • PR #846 Comment #2 — Info For dropdown центрирован, в спеке нет.
  • PR #847 — pageId формат с #faq-суффиксом (хотя Michael говорил "URL path of the page").
  • Уточнение по #847: page-level aggregation — не этот паттерн: подход согласован командой (Саша Федоренко, менеджер, Иван), это не «Иван в одиночку дополнил спеку».

Корневая причина: без явной фиксации в спеке/тикете и без согласования с командой и клиентом в реализацию попадают интерпретации и детали (пример — центрирование в #846). В #847 отдельно — несовпадение детали с формулировкой ревьюера (pageId / URL). Aggregation в #847 к этому паттерну не относится — она заранее согласована внутри команды.

Что лечит:

  • В сложных случаях — задавать вопрос до реализации, не после ревью.
  • Self-rule: "если в спеке этого нет — не делай. Хочешь сделать — спроси."

3. Self-QA пропуски (визуальные, responsive, accessibility, env-vars) (5 из 5 PR)

  • PR #837 — Contentful env разные.
  • PR #842 — console.log (Иван, фронт); отсутствие миграции 700 entries — scope Саши (Contentful App), не self-QA фронта.
  • PR #832 — undefined URL на проде, multiple hardcoded values, whitespace shuffles.
  • PR #846 — RWD регрессия 1024-1091, dark mode WCAG, не открыли expanded mobile.
  • PR #847 — dead env-var fallback (один grep по проекту).

Корневая причина: нет дисциплины самопроверки перед "Open PR".

Что лечит:

  • Self-QA checklist (ниже).

4. Невыученные уроки между PR

  • VideoObject без uploadDate — пойман Michael в PR #832 31.03 и в PR #842 31.03. Оба раза один день, оба раза комитил Иван. В PR #842 фронт-guard "если поле пустое — не выводить" не был реализован сразу; в PR #832 та же ошибка через ~5 недель повторилась. Между PR урок не закрепился. (NB: scope #842 формально на Саше — он спроектировал поле, но фронт-guard это уже на Иване.)
  • Spec-deviation — Bemin указывал на нарушение спеки в #837 (16.03), в #832 (24.02), в #846 (23.04). Ревьюеры системно ловят одни и те же классы ошибок.

Справка: 2026-03-05 Андрей письменно + в 1×1 уже поднимал с Иваном тему внимательности / «не на автопилоте» (по schema coverage doc — deliverable тогда был доведён). Не претензия, просто факт, что вопрос обсуждался ранее. См. .

Корневая причина: нет ретроспективы / разбора каждого замечания так, чтобы стало правилом.

Что лечит:

  • На встрече выбрать формат (можно начать с минимума и усилить позже):
    • A — журнал в репо: дополнять после PR с существенными замечаниями (класс ошибки + PR + одна строка «как не повторить»); перед новым похожим PR исполнитель перечитывает последние записи.
    • B — Jira: фиксация в комменте к parent / отдельная задача-«каталог повторов».
    • C — без отдельного журнала: короткий живой список в + ссылка из PR template на «типовые ловушки»; обновляет тот, кого назначит команда.
    • К любому из A–C (опционально): ИИ как помощник — например, в чат/инструмент закинуть тред ревью или краткое резюме PR, попросить черновик строки «класс ошибки + не повторять» или чеклист «что похожее уже ловили» перед стартом новой задачи; человек правит и утверждает перед записью в репо/Jira. Не замена ответственности, ускорение черновика и меньше шанс забыть класс замечания.
  • Владелец ритма (кто напоминает дописать строку, кто следит за актуальностью) — договорённость команды (тимлид / менеджер / ротация / исполнитель PR self-check перед Open) — не зашивать в материалах как «только тимлид».

5. Workflow дисциплина

  • PR #832 — Alexis уже 23.02 прямо спросил про pre-deployment steps; ответили "no additional steps required"; через 5 недель в PR #842 ровно эта же проблема повторилась — Michael узнал через сломанный build.
  • PR #847 — commit с TODO "when approve PR" → намеренный commit-after-approve.
  • PR #837/#847 — squashing на расшаренной ветке (TBD на встрече).
  • PR #846 — language barrier в комментариях ревью, потеря циклов на расшифровку.

Корневая причина: правила PR-workflow не формализованы / не проговорены явно.

Что лечит:

  • Свод правил workflow (ниже).
  • Менеджер/тимлид перечитывает существенные комментарии Ивана к ревьюерам перед публикацией.

Что в нашу пользу (плюсы для встречи со Stevens)

ПлюсГде
Реактивность Ивана — фиксит на следующий деньвсе 5 PR
Архитектурно правильное решениеPR #847 (aggregation > shortcut Michael)
QA с первого разаPR #837 (3/3 кейса)
Хорошее PR descriptionPR #847, #842 (после правок)
Merge вместо rebasePR #837 (по совету Alexis)
Branch reuse по предложению MichaelPR #847
Хороший вопрос с собственной инициативойPR #846 alignment magnifier/X
Спека сама требовала уточнений (3 случая)PR #846 (sticky 1024+, alignment, data-cta stability)

Системный вывод

Распределение замечаний по корневым причинам (по всем PR суммарно ~33 содержательных претензий):

  • ~70% — наши self-QA / spec-discipline промахи — лечатся checklist'ом + spec-read checkpoint.
  • ~15% — workflow violations (squashing, commit-after-approve, missing pre-merge tasks) — лечатся свод правил.
  • ~10% — пробелы в спеке у клиента (3 случая в PR #846) — это процессный gap на их стороне, аккуратно подсвечиваем.
  • ~5% — реюз существующих абстракций / codebase (пример PR #832 — getExternalPath, Image Wrapper.credit). Команда в проекте уже с конца февраля 2026 — «ещё только онбординг» как единственное оправдание слабее: к этому сроку можно было бы ориентироваться в типовых utilities (оценка доли грубая, на встрече можно пересмотреть). Сильнее лечится практикой перед фичей (поиск похожих мест, grep) и коротким ревью с упором на реюз, не отдельным бесконечным онбордингом.

Главное: проблема процессная, не индивидуальная. Иван — реактивный, технически достаточный разработчик. Что отсутствует — дисциплина пред-выпускной проверки и формализованный workflow. Это лечится за 1-2 итерации, если ввести checklist'ы и правила.

Предлагаемые процессные изменения

A. Self-QA checklist (Ивану — обязательно перед "Open PR")

  1. Spec sweep: открыт parent-тикет в Jira; прочитаны все attachments; acceptance criteria построчно сверены с реализацией.
  2. Mockup overlay: preview ↔ мокап разложены рядом; пройдены все состояния (default, sticky, hover, expanded, search open).
  3. Responsive: RWD-режим в DevTools, ползунок ширины через все breakpoint'ы (тестировать между ними, не только на стандартных).
  4. Mobile menu / overlays / dropdowns — открыты вручную хоть раз.
  5. Dark mode / accessibility: Lighthouse / axe DevTools / contrast check.
  6. Code hygiene: git diff пройден глазами; нет console.log, // TODO when approve, dead fallback'ов; grep env-vars подтверждает их существование.
  7. Schema/spec compliance: если выводится structured data — провалидирован через Rich Results / Fast Schema Analyzer.
  8. Existing data: если фича триггерится на новых данных — продумано, что с историческими данными.

B. PR description template (обязательные секции)

  • Summary — что и зачем
  • Changes — список изменений
  • Pre-merge tasks — что нужно сделать до merge (отдельные PR, Contentful changes, env-var-добавления и т.д.)
  • Migration plan — что с существующими данными, какой backfill
  • Validation done — какие тесты прогнаны, ссылки на preview-валидаторы
  • Compare — preview URL ↔ prod URL (если применимо)

C. Workflow rules (свод)

  1. После первого git push ветки на remote — никаких rebase / force-push / squash. Только новые коммиты сверху.
  2. Squash — только при финальном merge через UI ("Squash and merge" кнопка).
  3. Никаких "TODO when approve" в коде. Если код не готов — PR в Draft, не Open.
  4. После approve — изменения только с явным "please re-review". Approve привязан к коммиту.
  5. Branch reuse OK (как предложил Michael), но история не переписывается.

D. Spec-read checkpoint

  • Перед стартом задачи Иван пишет в Jira-комменте: "Read parent ICUS-X + attachments [список]. Acceptance criteria: [перечисляет]. Questions: [если есть]."
  • Антон/Андрей подтверждает или уточняет.

E. Lessons learned (формат — на встрече)

  • Черновик журнала в репо: — если команда выберет вариант A; иначе переносим смысл в Jira/wiki (вариант B) или короткий список в analysis + PR template (вариант C).
  • Базовый ритм (если журнал ведём): после замержа PR с существенными замечаниями — 1–2 строки «какой класс — как не повторить»; перед стартом следующего похожего PR исполнитель перечитывает записи (кто набросал первый черновик строки — не обязан быть только тимлидом: договорённость команды, см. паттерн 4).
  • Минимум независимо от варианта: после PR с замечаниями фиксировать класс ошибки (1–2 строки) и перед следующим похожим PR — явно перечитать (self-QA + эта «память»).
  • ИИ (пилот на усмотрение команды): тот же журнал или Jira можно подпитывать черновиками из ИИ (см. паттерн 4, опция к A–C) — в духе вопроса Alexis про AI-tooling, но в узком и безопасном scope: только текст ревью / описание, без секретов; всегда человеческая правка перед публикацией.
  • Кто ведёт и как напоминаем — решает команда на встрече (см. паттерн 4, «Что лечит»).

F. Размер батча

Предложение Андрея (мнение, не факт; нужно согласование с командой):

  • Если на старте видим, что задача большая — дробим заранее.
  • Если в процессе понимаем, что задача разрослась — дробим по ходу, не дожидаясь финала.
  • Зачем: чтобы не получить это как претензию от клиента в будущем (как сейчас).

Конкретно по PR #832: очевидно, что он разросся потому что разросся — не было намерения сделать огромный PR, просто фактически в одну линию работ попали схема + merge с master + правки вокруг next-seo. Не виним лично; фиксируем как урок для будущих похожих ситуаций. PR #846 (2 недели + 11 issues) — другой пример того же класса: большой scope = много циклов ревью.

Что прямо упомянуто в письме Alexis и наш ответ

Тезис письмаОтвет
"deviate considerably from spec"Признаём (5 из 5 PR). Вводим Self-QA checklist + Spec-read checkpoint.
"PR #847 commit squashing"TBD на встрече с Иваном (git log). Заводим правило "no rebase after share".
"internal validation pass prior to sending"Смысл тезиса Alexis закрываем Self-QA checklist (секция A) и секцией Validation done в описании PR — без отдельного обязательного gate «internal validation» менеджера/тимлида на каждый PR.
"ensure post-deployment tasks accompanied"Не делалось. Вводим обязательную секцию PR description.
"smaller batches, reduce TTM"Признаём. PR #832 был 2+ мес, PR #846 — 2 нед + 11 issues. Договоримся о делении.
"AI-assisted tooling"Открытый вопрос — обсудить с Иваном/Антоном.