PR #847 — итоговая позиция
PR: https://bitbucket.org/stevens_edu/stevens_main_nextjs/pull-requests/847
Связан с: PR #837 / branch ICUS-216-faq (переиспользование по предложению Michael)
Статус: OPEN, активная работа на 2026-04-30
Период: 2026-04-24 → 2026-04-30+
Особый вес
PR #847 прямо упомянут в письме Alexis (29.04) как пример commit squashing после расшаривания ветки. От разбора зависит позиция по 2-му пункту письма.
Сводка по существу
| # | Тема | Серьёзность | Статус |
|---|---|---|---|
| 0 | PR description (aggregation + @id) — архитектурно лучше shortcut Michael | ✓ плюс | — |
| 1 | bottomComponentsCollection пропущен (Right Nav, Program Detail) | выс. | Resolved |
| 2 | Vercel deploy bot | служ. | — |
| 3 | Commit с TODO "remove when approve PR" → commit-after-approve | крит. | Resolved по требованию |
| 4 | NEXT_PUBLIC_SITE_URL dead fallback | сред. | TBD |
| 5 | Активность today (2 коммита) | служ. | — |
Что в нашу пользу
- Архитектурное решение лучше, чем shortcut Michael: page-level aggregation +
@id. Подход aggregation — согласован командой (Саша Федоренко, менеджер, Иван); в PR реализация и описание — Ивана; технически это опровергло тезис Michael "merge would be quite difficult". - Branch reuse — по прямой рекомендации Michael ("multiple PRs for the same branch is no problem at all"). Эту часть критики Alexis (если окажется про branch reuse) — отбиваем.
- PR description хорошо оформлен — Summary, Changes, Compare URLs, preview-валидация через Fast Schema Analyzer.
- Иван реактивно правит — на каждое замечание есть исправляющий коммит.
Что признаём (без оговорок)
Главное — commit-after-approve
Иван закоммитил закомментированный код с TODO-инструкцией:
"remove and uncomment const questions when approve PR"
Это не недопонимание и не пробел спеки. Это сознательное намерение изменить код после approve — что нарушает базовое правило PR-workflow ("approve привязан к конкретному коммиту"). Michael это прямо зафиксировал:
"The sequence we use to ensure accurate approvals is {commit → approve → merge}, not {commit → approve → commit → merge}."
Это не защищается. Признаём, чиним процесс.
Прочие признания
bottomComponentsCollectiongap — Иван в description сам обещал покрыть all 4 типа страниц, в коде — нет. Self-QA fail.NEXT_PUBLIC_SITE_URL— dead fallback, проверяется одним grep'ом по проекту перед открытием PR.
Что отложено на встречу с Иваном (завтра)
⏳ Git-история (squashing/rebase/force-push) — мы без локального репо не проверим. Чеклист в (Приоритет 2 — squashing/git-история).
Связь с письмом Alexis
| Тезис письма | Что нашли в PR #847 |
|---|---|
| "avoid rebasing/squashing after code is shared" | TBD до завтрашней встречи (git log) |
| "ensure accurate approvals" (косвенно — через "team has to review all of it once more") | Найден более серьёзный кейс — commit-after-approve schema. Усиливает претензию Alexis. |
| "deviate from spec" | bottomComponentsCollection gap (description обещает шире, чем код) |
| "internal validation pass" | dead env-var fallback, gap покрытия |
Главные тезисы для встречи
Со Stevens (через Tom)
- По существу PR хороший — архитектурно правильное решение.
- Branch reuse был по совету Michael — это OK.
- Признаём commit-after-approve как нарушение — заводим правило: никаких "TODO when approve" в коде, никаких изменений в коде после approve без re-request review.
- По squashing — установим факты с разработчиком и вернёмся (если git log что-то покажет).
Вопросы по конкретике (фактическая часть — открываем Bitbucket вместе)
- Понимание команды: approve привязан к коммиту. Что значил TODO «when approve PR» — действительно планировалось менять код после approval? Если да — почему не draft-PR?
- Тестировался ли right-nav/program-detail с FAQ в bottom-области?
NEXT_PUBLIC_SITE_URL— почему fallback на несуществующую переменную?- Git-история: squash при PR #837? force-push? локальный rebase? — смотрим в Bitbucket commits page и
git reflog ICUS-216-faq.
Командные правила, которые предлагаем зафиксировать
- Никаких "TODO when approve" в коде, попадающем в PR. Если код не готов — Draft PR.
- После approve — изменения только с явной просьбой re-review.
- Self-QA checklist (mockup-overlay, responsive, dark mode, env-vars grep, тесты на всех декларированных типах страниц).
Ключевой вывод
По существу PR #847 — самый качественный из разобранных (архитектурно правильное решение, хорошее описание). По процессу — содержит самое серьёзное процессное нарушение во всей истории (commit-after-approve schema).
Это парадокс, который и нужно вынести на встречу: проблема не в технических навыках Ивана, а в дисциплине PR-workflow и self-QA. Это процессно лечится.