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 spec | Comment #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 such | Comment #2: Alexis ещё 23 февраля прямо спрашивал про pre-deployment steps. То есть проблема была озвучена за 2 месяца до письма и не решена системно. |
| internal validation pass prior to sending | Все 9+ замечаний — это то, что должен ловить self-review / lint / тимлид-проверка перед отправкой ревьюерам. Особенно undefined URL, который уже виден на проде (preview). |
| smaller batches, reduce TTM | 2+ месяца, ≥27 коммитов в одном PR, REQUESTED CHANGES, ~9 батчей апдейтов |
Сводка по комментариям
| # | Автор | Серьёзность | Тема | Обоснованно |
|---|---|---|---|---|
| 1 | Bemin | сред. | Minisite Right Nav pages не включены | ✓ Да |
| 2 | Alexis | крит. | Hold + прямой запрос pre-deployment steps | ✓ Да — прямой эхо письма за 2 мес |
| 3 | Bemin | observ. | Contact pages PostalAddress в rich text | Observation/блокер модели |
| 4 | Michael | крит. | Invalid VideoJsonLd для hero (REQUESTED CHANGES) | ✓ Да — повторение ошибки PR #842 |
| 5 | Michael | сред. | Whitespace-only изменения, конфликт с другим PR | ✓ Да |
| 6 | Bemin | крит. | undefined base URL → "url":"undefined/profile/..." на проде | ✓ Да — функциональный баг |
| 7 | Bemin | сред. | Hardcoded /news/ префикс вместо getExternalPath | ✓ Да |
| 8 | Bemin | сред. | Hardcoded creditText (events) | ✓ Да |
| 9 | Bemin | сред. | 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.
Корневые причины
- Отсутствие self-review дисциплины — undefined URL, debug-код, hardcoded строки, лишние whitespace-изменения — это всё ловится за 5-15 минут перед открытием PR.
- Слабое знание codebase —
getExternalPath,Image Wrapper.creditуже есть, но Иван их не использовал. - Невыученные уроки между PR — invalid VideoObject повторился из PR #842 в #832.
- Отсутствие internal validation pass — большинство багов должно ловиться нашей стороной до отправки на ревью.
- Слишком крупный батч работы — 27+ коммитов в одном PR, охват широкий, цикл ревью затянулся; происхождение см. §5 выше (схема + затем merge/
next-seo). Это не железная «претензия» в адрес намерения, но факт нагрузки на ревью остаётся — вывод процессный, не моральный. - Сигнал не закрепился у исполнителя 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/creditfield — не знал об их существовании или забыл? - Почему повторил ошибку invalid VideoObject из PR #842?
- Командный процесс — что меняем: как после менеджерского Slack и комментария в PR получать явное подтверждение от исполнителя PR, что требование услышано и проверено? Делается ли self-review / internal validation pass на нашей стороне? Почему такой большой PR (2+ месяца, 27+ коммитов) — была ли возможность разбить? Подтвердить историю «IX-SIT60 + потом merge/
next-seoв ту же ветку» и зафиксировать правило на будущее.
Ключевой вывод
PR #832 — это документально подтверждённая иллюстрация того, что:
- Проблемы, на которые жалуется Alexis в письме, — не новые. Они звучали явно ещё в феврале.
- Эти проблемы системные (повторяются между PR), не разовые.
- Исполнитель PR (Иван) не закрепил ранние сигналы (PR + Slack) до смены практики; с Антоном — выяснить процессный слой (подтверждение «услышал» после эскалаций), без линии «тимлид не знал».
Для встречи: на базе этого PR честная позиция — признать, что мы пропустили ранние сигналы и не отстроили процессы. Дальнейший разговор должен быть не о защите, а о конкретном плане, как это исправить (PR template, self-review checklist, разбиение работы, internal validation pass — и кто за это отвечает).
В разговоре с клиентом этот PR лучше не выводить как пример — он играет против нас сильнее всех остальных. Использовать его внутри нашей команды как материал для разговора с Антоном/Иваном.