Retro

PR #837 — комментарии и разбор

PR: https://bitbucket.org/stevens_edu/stevens_main_nextjs/pull-requests/837 Задача: ICUS-216 — Add FAQPageJsonLd for accordion

Формат: для каждого комментария — скриншот, автор, цитата, наш разбор.

Таймлайн PR

  • 2026-03-13 (пт): Иван открыл PR
  • 2026-03-16 (пн): комментарий Bemin Shaker про Contentful master vs develop
  • 2026-03-17 (вт): ответ Ивана — "поля убраны со staging и перенесены в development branch"
  • 2026-03-17 (вт): автокомментарий Vercel for Bitbucket (от имени Michael Forbes) — деплой готов, preview доступен
  • 2026-03-17 (вт): Иван запушил 1 коммит 3cf69c0 (UPDATED)
  • 2026-03-16 (пн): комментарий Michael Forbes — next-seo 7.2.0 уже в master, рекомендация смержить master → ICUS-216-faq
  • 2026-03-17 (вт): Иван — "I'll update the branch right now" → "Updated" → отредактировал description
  • 2026-03-17 (вт): Michael Forbes опубликовал результаты QA-тестирования (3 кейса: Yes / No / null) — The test passes, JSON-LD выглядит корректно
  • 2026-03-19 (чт): Michael — повторный тест после изменения конфигурации поля (FAQ вместо Yes, Standard Content вместо No) — тест по-прежнему проходит
  • 2026-03-17 (вт): Michael — "code looks great, but field configuration needs adjustments to match Tom's specs" (5 пунктов + рекомендация сделать Required без default)
  • 2026-03-19 (чт): Иван — "A new property has been added, and its behavior has been updated. Everything is just as you described."
  • 2026-03-19 (чт): Иван отредактировал description PR
  • 2026-03-19 (чт), 18:02 UTC: последний апдейт деплоя Vercel
  • 2026-03-19 (чт): Michael Forbes добавил 2 ревьюеров: AW (Alexis Watson?) и BS (Bemin Shaker)
  • 2026-03-23 (пн): Bemin Shaker — APPROVED
  • 2026-03-24 (вт): Bemin Shaker — MERGED

Итоговый цикл PR: 11 дней (13 марта → 24 марта).

Comment #1 — Contentful: изменения content model в master вместо develop

  • Скриншот:

  • Автор: Bemin Shaker (Stevens / iX)

  • Дата: 2026-03-16

  • Контекст: новое поле "Enable FAQ Schema" в Contentful

  • Кому копать внутри CPCS: операционный ответ «куда и как вносить изменения content model» для таких кейсов — скорее к Саше Федоренко (Contentful / модель контента). При этом на внутренней встрече тему стоит поднять со всей командой: у клиента реакция накопительная (не только этот PR), нужно утвердить, как поступаем дальше — единое правило, ответственность и чеклист в тикете/PR.

  • Цитата:

    I noticed the content model change for this (the new Enable FAQ Schema field) was made in master rather than develop. Ideally, content model changes should never be made in the production environment before they are approved, for a number of reasons. For instance, it may create confusion for content editors about whether the feature has actually been deployed. To properly test these changes, we would need to make the content model updates in the develop Contentful environment, which is what any non-master branch uses automatically. Another option would be to create a separate issue-specific Contentful environment and configure the branch to use it through environment variables.

  • Суть замечания: Иван внёс изменение content model (поле "Enable FAQ Schema") в production-окружение Contentful (master), а не в develop. Это:

    1. Может ввести в заблуждение контент-редакторов (поле уже видно в проде, но фича ещё не задеплоена).
    2. Ломает процесс тестирования: non-master ветки автоматически смотрят на develop Contentful — а там поля нет.
    3. Альтернативно можно было создать issue-specific Contentful environment и через env vars прицепить ветку к нему.

  • Ответ Ивана (2026-03-17):

    The fields were removed from the staging environment and moved to the development branch.

    (Поля убраны со staging-окружения и перенесены в development branch.)

  • Обоснованно? (да/частично/нет): Да. Это процессная ошибка — менять content model в production Contentful до утверждения фичи действительно неправильно. Bemin корректно описал проблему и сразу предложил два решения.

  • Терминологический нюанс:

    • В письме Bemin'а: master = production, develop = dev.
    • В ответе Ивана упомянут "staging environment" — нужно уточнить: имел ли он в виду master (production) или действительно отдельный staging? Возможна путаница в терминах.

  • Наша позиция / для встречи:

    • Признать процессную ошибку: content model менялся не в той среде.
    • Иван оперативно (на следующий день) исправил.
    • Вопросы к Ивану:
      • Понимал ли он разницу между master / develop / staging Contentful environments на момент изменения?
      • Есть ли где-то документация/чеклист "куда какие изменения вносить" для Contentful?
      • Уточнить терминологию в его ответе (staging vs master).
    • Вопрос к Антону / процессу:
      • Как формализовать правило "никогда не трогать master Contentful до approve"? Onboarding / checklist / автоматизация?
      • Имеет ли смысл практика issue-specific Contentful environments, которую предлагает Bemin?
    • Саша Федоренко: подтвердить фактическую процедуру в Contentful (кто меняет модель, в каком env первым шагом, кто даёт «ок» перед prod).

Comment #2 — Vercel for Bitbucket (автобот, не претензия)

  • Скриншот:

  • Автор: Michael Forbes (по факту — автоматический комментарий Vercel for Bitbucket, постится от его аккаунта/токена)

  • Дата: 2026-03-17

  • Содержание:

    • Project: stevens-site-redesign
    • Deployment: Ready (зелёный)
    • Review: Preview / Comment ссылки
    • Updated (UTC): Mar 19, 2026 6:02pm

  • Это претензия? Нет. Это служебное сообщение интеграции Vercel ↔ Bitbucket: deploy preview успешно собрался.

  • Зачем фиксируем: для таймлайна — видно, что после комментария Bemin Иван запушил исправление (3cf69c0), деплой собрался успешно. Полезно как доказательство оперативной реакции.

  • Действий не требуется.

Comment #3 — Michael Forbes: смержить master в ветку (next-seo 7.2.0 в проде)

  • Скриншот:

  • Автор: Michael Forbes (живой комментарий, не бот)

  • Дата: 2026-03-16

  • Цитата:

    @Ivan.Kotovsky We now have next-seo 7.2.0 in production (the PR was merged into the master branch) so I recommend that you merge master into ICUS-216-faq and also commit the changes you mentioned in the comment. Thanks!

  • Суть:

    • Зависимость next-seo обновлена до 7.2.0 в проде (другой PR смержен в master).
    • Michael рекомендует Ивану влить master → ICUS-216-faq, чтобы подтянуть новую версию + закоммитить ранее обещанные изменения.
    • Это не претензия, а координационный совет по зависимости.
    • Тон дружелюбный (Thanks!), стоит лайк.

  • Реакция Ивана (2026-03-17):

    That's cool, I'll update the branch right now.

    Updated

    → обновил ветку и отредактировал описание PR.

  • Это претензия? Нет. Координация по зависимостям между PR.

  • Что важно для нашего разбора:

    • Иван смержил master в свою ветку (а не сделал rebase) — это как раз соответствует пожеланию Stevens из письма Alexis ("avoid rebasing after code is shared"). Плюс в нашу пользу: история сохранена.
    • Реакция оперативная (на следующий день).
    • Можно использовать как пример хорошего workflow в этом конкретном PR — в противовес общим претензиям.

  • Действий не требуется, но фиксируем как позитивный кейс для встречи.

Comment #4 — Michael Forbes: результаты QA-тестирования (positive)

  • Скриншоты:

    • — описание 3 тест-кейсов
    • — JSON-LD ч.1 (Workday FAQ, начало)
    • — JSON-LD ч.2 (Workday FAQ, окончание)
    • — повторный тест после изменения значений

  • Автор: Michael Forbes

  • Дата: 2026-03-17, повторный тест 2026-03-19

  • Содержание (2026-03-17):

    For testing...

    • I set the new field to Yes on this entry (Workday FAQ): Before / After (should have FAQ schema *)
    • I set the new field to No on this entry (Leadership FAQ): Before / After (should not have FAQ schema)
    • I left the new field null on this entry (Co-op FAQ): Before / After (should not have FAQ schema)

    The test passes.

    *Here is the json (abbreviated for this comment), which looks good: [JSON-LD FAQPage с вопросами про Workday]

    Michael вручную прогнал три кейса:

    Значение поляEntryОжидаемый результатФакт
    YesWorkday FAQFAQ schema присутствует✓
    NoLeadership FAQFAQ schema отсутствует✓
    nullCo-op FAQFAQ schema отсутствует✓

    JSON-LD валидный: @context: schema.org, @type: FAQPage, корректный массив mainEntity с Question/Answer.

  • Содержание (2026-03-19):

    The recent adjustment to the field configuration led me to perform this test again, setting a value of FAQ instead of Yes and Standard Content instead of No. The test still passes.

    После того, как значения поля переименовали (Yes→FAQ, No→Standard Content), Michael перепрогнал тесты — снова всё проходит.

  • Это претензия? Нет. Это позитивный результат QA со стороны Stevens/iX.

  • Что важно для нашего разбора (плюс в нашу копилку):

    • Реализация Ивана прошла QA с первого раза во всех трёх сценариях.
    • JSON-LD соответствует schema.org spec (FAQPage / Question / Answer) — спека выдержана.
    • После переименования значений поля (запросили Stevens) — всё продолжило работать, регрессии нет.
    • Это сильный контраргумент к общей претензии "deviate considerably from spec" из письма Alexis: на этом конкретном PR функциональность отвечает спеке.

  • Действий не требуется. Использовать как пример успешного PR на встрече.

Comment #5 — Michael Forbes: код ок, но конфигурация поля не соответствует спеке Тома - Скриншот:

  • Автор: Michael Forbes

  • Дата: 2026-03-17

  • Цитата:

    The code looks great, but the field configuration needs some adjustments to match the specs provided by Tom:

    1. Accordion content type includes a new field:
      • Field name: Accordion Purpose
      • Type: Single select
      • Options: Standard Content (default), FAQ
    2. Default value = Standard Content.
    3. FAQ schema is generated only when: Accordion Purpose = FAQ
    4. Existing Accordion instances default to: Standard Content (no retroactive schema without author action)
    5. Help text displayed under "Accordion Purpose": "Select FAQ only when content is a list of user-facing questions and answers."

    However, I recommend a small adjustment: we should not set a Default value at all, and we should make the field Required. This will be very beneficial, because a Contentful user working on an Accordion entry will be unable to publish the entry until they actually make a decision about this field. If it's not Required (or if it offers a Default value) then we are allowing the Contentful user to ignore the field instead of truly making a decision, which increases the likelihood of a mistake (such as writing FAQ content but noticing they should set this field to FAQ).

  • Суть замечания: Сам код Иван написал хорошо ("code looks great"), но конфигурация поля в Contentful не соответствовала спецификации, которую предоставил Tom (iX-менеджер). Расхождения:

    • Имя поля было другое (то ли Enable FAQ Schema, то ли что-то иное → должно быть Accordion Purpose).
    • Тип/значения были boolean-подобные (Yes/No из предыдущих комментариев) → должны быть Single select со значениями Standard Content (default) и FAQ.
    • Логика триггера schema должна быть привязана к значению FAQ.
    • Existing instances должны фолбечиться на Standard Content.
    • Нужен help-text.

    Плюс дополнительная рекомендация Michael: не ставить default, сделать поле Required — чтобы редактор был вынужден осознанно выбирать значение.

  • Реакция Ивана (2026-03-19):

    A new property has been added, and its behavior has been updated. Everything is just as you described.

    → выполнил все 5 пунктов + рекомендацию по Required.

  • Обоснованно? (да/частично/нет): Да, абсолютно. Прямое расхождение с зафиксированной спекой ICUS-214 (см. ). Michael в комментарии цитирует acceptance criteria дословно из User Story 1.0 и User Story 3.0 — это не субъективная рекомендация, а формально утверждённые требования.

  • ВАЖНО для общего разбора: Это прямой пример того, о чём пишет Alexis в письме: "deliverables that deviate considerably from spec and require revisiting". Здесь именно такой кейс:

    • Иван реализовал не по спеке Тома, а по своему пониманию (Yes/No вместо single-select Standard Content/FAQ).
    • Michael был вынужден ловить это на ревью и подробно расписывать что нужно.
    • QA-тестирование (Comment #4) пришлось перепрогонять после исправления.

    С другой стороны:

    • Расхождение не катастрофическое — функционально работало, JSON-LD генерировался корректно (это подтвердил тот же Michael в Comment #4).
    • Иван оперативно исправил (2 дня) и подтвердил соответствие.

  • Критичное обстоятельство: Спецификация была приложена к нашей родительской Jira-задаче ICUS-214 в виде PDF "User Stories for FAQ Schema - Copy.pdf":

    • Приложил: Andrii Holubets (менеджер)
    • Когда: 2026-03-11, 16:34 Europe/Warsaw — за 2 дня до открытия PR (13.03)

    См. .

    Все 5 пунктов, которые Michael попросил исправить, дословно взяты из acceptance criteria User Story 1.0 и User Story 3.0:

    • Field name: Accordion Purpose ✓
    • Type: Single select ✓
    • Options: Standard Content (default), FAQ ✓
    • Default = Standard Content ✓
    • FAQ schema only when = FAQ ✓
    • Existing instances default to Standard Content ✓
    • Help text дословно ✓

    Это значит: спека была доступна в нашей собственной Jira до старта разработки. Это не вопрос "не дошла спека". Это вопрос — спека не была прочитана / понята / выполнена разработчиком.

  • Открытые вопросы:

    • Иван — открывал ли ты ICUS-214 и читал ли приложенный PDF до старта работы по ICUS-216?
    • Если открывал — почему отступил от 5 acceptance criteria, которые там явно прописаны?
    • Если не открывал — почему? Это привычка работать только по сабтаску без чтения parent-тикета?
    • Антон — есть ли у нас правило "перед стартом сабтаска прочитать parent-story и приложенные спеки"? Кто это контролирует?

  • Наша позиция / для встречи (пересмотренная после нахождения спеки):

    • Признать полностью: расхождение со спекой не косвенное и не из-за плохой передачи требований. Спека была в нашей же Jira (ICUS-214), приложена в виде PDF, и не была выполнена.
    • Смягчающие:
      • Код качественный (Michael: "code looks great").
      • JSON-LD валидный.
      • QA после исправления прошёл с первого раза.
      • Иван оперативно (за 2 дня) поправил всё, что сказал Michael.
    • Корневая причина (честная):
      • Не "спека не дошла" — спека лежала в parent-тикете.
      • А скорее "разработчик начал кодить, не прочитав спеку детально" + "менеджмент не проверил соответствие спеке до отправки PR".
    • Что предлагаем менять:
      1. Чеклист "Spec read & understood" перед стартом сабтаска — Иван явно подтверждает, что прочитал acceptance criteria parent-стори.
      2. Перед открытием PR на ревью Stevens — internal validation pass: тимлид/менеджер сверяет реализацию с acceptance criteria построчно (это и есть то, что просит Alexis в письме).
      3. Для Contentful-полей — таблица "field name / type / options / default / required / help text" обязательной частью описания PR (или тикета).
<!-- добавлять блоки по мере поступления скриншотов -->