Retro

PR #832 — итоговая позиция

PR: https://bitbucket.org/stevens_edu/stevens_main_nextjs/pull-requests/832 Задача: IX-SIT60 — Add schema for pages Спека в задаче: с 2026-02-17📄Stevens_Schema_Templates.odt. Старт работ: 2026-02-19. PR открыт 2026-02-21 (+2 дня после старта). Статус: OPEN (на момент скринов, ~2026-04-24, Vercel last deploy) Цикл: 2026-02-21 → 2026-04-24+ = 2+ месяца Объём: ≥27 коммитов в 9+ батчах апдейтов REQUESTED CHANGES: да (Michael Forbes, 31.03)

Это самый проблемный PR из разобранных

PR #832 концентрирует все 4 пункта письма Alexis в одном артефакте:

Претензия письма (29.04)Прямое доказательство в PR #832
deviate considerably from specComment #1 (Minisite Right Nav пропущен), Comment #4 (invalid VideoObject — повторение ошибки из PR #842), Comment #6 (undefined base URL в проде), Comment #7 (hardcoded /news/ вместо getExternalPath), Comment #8/9 (hardcoded creditText в 2 местах)
PRs require post-deployment tasks accompanied by suchComment #2: Alexis ещё 23 февраля прямо спрашивал про pre-deployment steps. То есть проблема была озвучена за 2 месяца до письма и не решена системно.
internal validation pass prior to sendingВсе 9+ замечаний — это то, что должен ловить self-review / lint / тимлид-проверка перед отправкой ревьюерам. Особенно undefined URL, который уже виден на проде (preview).
smaller batches, reduce TTM2+ месяца, ≥27 коммитов в одном PR, REQUESTED CHANGES, ~9 батчей апдейтов

Сводка по комментариям

#АвторСерьёзностьТемаОбоснованно
1Beminсред.Minisite Right Nav pages не включены✓ Да
2Alexisкрит.Hold + прямой запрос pre-deployment steps✓ Да — прямой эхо письма за 2 мес
3Beminobserv.Contact pages PostalAddress в rich textObservation/блокер модели
4Michaelкрит.Invalid VideoJsonLd для hero (REQUESTED CHANGES)✓ Да — повторение ошибки PR #842
5Michaelсред.Whitespace-only изменения, конфликт с другим PR✓ Да
6Beminкрит.undefined base URL → "url":"undefined/profile/..." на проде✓ Да — функциональный баг
7Beminсред.Hardcoded /news/ префикс вместо getExternalPath✓ Да
8Beminсред.Hardcoded creditText (events)✓ Да
9Beminсред.Hardcoded creditText (program)✓ Да — повтор ошибки внутри PR

Всего: 9 значимых замечаний, 8 из 9 обоснованы, 3 — критичные (включая REQUESTED CHANGES).

Критичные открытия

1. Alexis сигнализировал про pre-deployment steps уже 23 февраля (Comment #2)

Письмо от 29.04 — это не первый сигнал. Alexis в PR #832 2 месяца назад прямо спрашивал про pre-deployment steps (Comment #2): "are there any additional steps we need to take prior to deployment? This would include any changes to Contentful fields and the like." Иван ответил "no additional steps". Это значит:

  • Проблема была явно поднята.
  • Внутренний контекст: я (Андрей) несколько раз поднимал эту тему — говорил и писал в Slack, что нас панчат за это. Точные даты найти не получилось.

Это ключевой тезис для встречи: мы не можем говорить "только что узнали из письма". Это уже звучало — и дублировалось в Slack.

2. Повторяющаяся ошибка про invalid VideoObject (Comment #4)

В PR #842 Comment #3 Michael ловил это: VideoObject без uploadDate невалиден. В PR #832 Comment #4 (хронологически — раньше, в PR #832 31.03; PR #842 ту же ошибку 31.03 тоже) тот же Michael ловит ту же ошибку в коде Ивана. Это паттерн, не случайность — Иван не усвоил это требование между PR.

3. Undefined URL уже на production preview (Comment #6)

"url":"undefined/profile/fkim" — Bemin видел на проде (preview). Это значит:

  • Тестирования / self-review реально не было.
  • Поломанная функциональность дошла до этапа ревью клиентом.

4. Использование существующих абстракций

Comment #7, #8, #9 — Иван не использует getExternalPath и Image Wrapper.credit field, хотя они уже есть в codebase. Это вопрос знания проекта и внимательности к существующим решениям.

5. Огромный размер и длительность PR

2+ месяца, ≥27 коммитов, ≥9 батчей апдейтов — по факту совпадает с мыслью Alexis про smaller batches (в письме это предложение, не отдельная формулированная претензия). Для ревьюера всё равно тяжело: широкий охват и длинный цикл ревью.

Контекст для внутренней ретро (не отменяет эффекта для ревью): объём исторически сложился — сначала одна задача по схеме для страниц, затем в ту же линию работ попали merge с master и правки вокруг next-seo / платформенные обновления. Это не обязательно осознанное решение «держать всё одним PR», но стоит обсудить на встрече: как дробить, когда в фичу вторгается dependency/platform work (отдельный PR, явное описание merge-шума в description, лимит времени OPEN).

Что в нашу пользу

  • Иван реагирует на каждое замечание. На все комментарии есть исправляющие коммиты.
  • PR не заброшен — постоянная активность.
  • На большинство стилистических/процессных замечаний (whitespace, hardcode) Иван не возражает, исправляет.

Однако этих смягчающих обстоятельств намного меньше, чем в PR #837 (где код "looks great") или PR #842.

Корневые причины

  1. Отсутствие self-review дисциплины — undefined URL, debug-код, hardcoded строки, лишние whitespace-изменения — это всё ловится за 5-15 минут перед открытием PR.
  2. Слабое знание codebasegetExternalPath, Image Wrapper.credit уже есть, но Иван их не использовал.
  3. Невыученные уроки между PR — invalid VideoObject повторился из PR #842 в #832.
  4. Отсутствие internal validation pass — большинство багов должно ловиться нашей стороной до отправки на ревью.
  5. Слишком крупный батч работы — 27+ коммитов в одном PR, охват широкий, цикл ревью затянулся; происхождение см. §5 выше (схема + затем merge/next-seo). Это не железная «претензия» в адрес намерения, но факт нагрузки на ревью остаётся — вывод процессный, не моральный.
  6. Сигнал не закрепился у исполнителя PR — Alexis 23.02 в PR + я (Андрей) несколько раз поднимал тему в Slack (что нас панчат, точные даты найти не получилось); ответ Ивана был «no additional steps», затем в PR #842 та же проблема повторилась. Обсудить процесс: как такие сигналы должны фиксироваться у исполнителя PR (явное «услышал/проверил»).

Действия / план до встречи

  • «Schema templates doc» / спека по типам страниц: 📄Stevens_Schema_Templates.odt в задаче с 17.02, работа с 19.02отсутствие спеки не драйвер для разбора и встречи. Замечание Bemin (Comment #1) — про покрытие типов в PR, не про поиск документации.
  • Уточнить у Ивана:
    • Comment #2 / pre-deployment: тему я (Андрей) несколько раз поднимал в Slack — говорил, что нас панчат за это (точные даты не нашёл). Что нужно изменить в процессе, чтобы такие напоминания закреплялись?
    • Почему ответил "no additional steps required" Alexis 24.02 — была ли действительно проверка?
    • Почему не использовал getExternalPath / credit field — не знал об их существовании или забыл?
    • Почему повторил ошибку invalid VideoObject из PR #842?
  • Командный процесс — что меняем: как после менеджерского Slack и комментария в PR получать явное подтверждение от исполнителя PR, что требование услышано и проверено? Делается ли self-review / internal validation pass на нашей стороне? Почему такой большой PR (2+ месяца, 27+ коммитов) — была ли возможность разбить? Подтвердить историю «IX-SIT60 + потом merge/next-seo в ту же ветку» и зафиксировать правило на будущее.

Ключевой вывод

PR #832 — это документально подтверждённая иллюстрация того, что:

  1. Проблемы, на которые жалуется Alexis в письме, — не новые. Они звучали явно ещё в феврале.
  2. Эти проблемы системные (повторяются между PR), не разовые.
  3. Исполнитель PR (Иван) не закрепил ранние сигналы (PR + Slack) до смены практики; с Антоном — выяснить процессный слой (подтверждение «услышал» после эскалаций), без линии «тимлид не знал».

Для встречи: на базе этого PR честная позиция — признать, что мы пропустили ранние сигналы и не отстроили процессы. Дальнейший разговор должен быть не о защите, а о конкретном плане, как это исправить (PR template, self-review checklist, разбиение работы, internal validation pass — и кто за это отвечает).

В разговоре с клиентом этот PR лучше не выводить как пример — он играет против нас сильнее всех остальных. Использовать его внутри нашей команды как материал для разговора с Антоном/Иваном.