137 lines
12 KiB
Markdown
137 lines
12 KiB
Markdown
---
|
||
name: phase-review
|
||
description: Ревью срезов и фаз проекта h-school по их же критериям приёмки — с дописыванием недостающих тестов, исправлением найденного и журналом проверенного в docs/phases/reviewed.md. Обязательно используй этот скилл, когда просят «сделай ревью», «проведи ревью», «проверь срез», «проверь непроверенные срезы», «что ещё не проверено», «review the slice» — и вообще при любой просьбе проверить, действительно ли сделанная фаза сделана. Не для ревью одного диффа или пул-реквеста: там нужен /code-review.
|
||
---
|
||
|
||
# Ревью срезов
|
||
|
||
Фазы в `docs/phases/` помечаются ✅, когда их **написали**. Это заявка автора, а не
|
||
доказательство. Скилл превращает заявку в проверенный факт: берёт срез, сверяет каждое его
|
||
обещание с кодом и тестами, дописывает недостающее, чинит найденное и записывает результат,
|
||
чтобы одну и ту же работу не делать дважды.
|
||
|
||
Главное, что делает ревью здесь дешёвым: **критерии приёмки уже написаны**. У каждой фазы есть
|
||
«Задачи», «Тесты, без которых фаза не закрыта» и «Критерий готовности», а у проекта — список
|
||
инвариантов в `AGENTS.md`. Ничего не надо выдумывать, надо проверить.
|
||
|
||
## Журнал
|
||
|
||
`docs/phases/reviewed.md` — единственный источник правды о том, что уже проверено. Если файла
|
||
нет, создай его с заголовком и одной строкой о том, что это такое.
|
||
|
||
Раздел на срез:
|
||
|
||
```markdown
|
||
## Срез 4. Расписание
|
||
|
||
- **Фазы:** 14–17
|
||
- **Проверен на:** `3c54f98`, 2026-08-19
|
||
- **Пути:** `src/HSchool.Schedule`, `src/HSchool.Simulation/SchoolTimetables.cs`,
|
||
`src/HSchool.Server/Api/TimetableEndpoints.cs`, `tests/HSchool.Schedule.Tests`
|
||
- **Итог:** дописано 3 теста, исправлено 2 расхождения с дизайном, одно замечание оставлено
|
||
открытым (см. ниже)
|
||
```
|
||
|
||
Коммит в строке «Проверен на» — не украшение. Он делает журнал самопроверяющимся: срез,
|
||
проверенный на `3c54f98`, перестаёт быть проверенным, как только его пути тронули дальше. Поэтому
|
||
«Пути» тоже обязательны — без них дрейф не поймать.
|
||
|
||
## Как выбрать, что проверять
|
||
|
||
1. Прочитай `docs/phases/README.md` — там срезы, их фазы и статусы.
|
||
2. Прочитай журнал.
|
||
3. Срез попадает в очередь, если:
|
||
- его нет в журнале; **или**
|
||
- `git log <коммит-из-журнала>..HEAD -- <пути среза>` непустой — код менялся после проверки;
|
||
это перепроверка, и в отчёте её надо называть именно так;
|
||
- в журнале записано «проверен частично».
|
||
4. Порядок — **от раннего к позднему**. Верхние срезы стоят на нижних: искать причину странного
|
||
расписания, не проверив генерацию людей, значит искать не там.
|
||
|
||
Срезы в работе (есть ⬜ или 🔄, код уже пишется) проверять можно и нужно, но в журнале это
|
||
отмечается как «в работе на момент проверки» — иначе следующий проход решит, что там всё
|
||
закрыто.
|
||
|
||
**Один срез за проход.** Проверил → починил → записал в журнал → доложил → взялся за следующий.
|
||
Если проходов будет несколько, пользователь увидит промежуточные результаты и сможет вмешаться,
|
||
а не получит через час одну кучу правок в семи проектах.
|
||
|
||
## Что именно проверять
|
||
|
||
Идти сверху вниз, по каждому пункту фазы отдельно:
|
||
|
||
**1. Обещания фазы.** Для каждой задачи и каждого пункта «Тесты, без которых фаза не закрыта»
|
||
найди фактическое подтверждение: строчку кода, которая это делает, и тест, который это
|
||
проверяет. Галочка `[x]` подтверждением не является. Чаще всего расхождение выглядит так: код
|
||
написан, а теста из списка нет — либо тест есть, но проверяет соседнее.
|
||
|
||
**2. Инварианты `AGENTS.md`,** относящиеся к затронутым проектам. Особенно те, что нельзя
|
||
нарушить незаметно: протокол в трёх местах сразу, отсутствие HTTP и ASP.NET в `Simulation` и в
|
||
чистых библиотеках, фиксированный шаг вместо `DateTime.Now`, одна школа — один поток,
|
||
детерминированность от сида.
|
||
|
||
**3. «Things that will bite you».** Этот раздел `AGENTS.md` — список уже случившихся регрессий.
|
||
Проверить, что ни одна не вернулась, дешевле, чем поймать её второй раз.
|
||
|
||
**4. Дизайн-док среза** (`docs/design/*.md`, ссылка есть в индексе фаз). Расхождение кода с
|
||
принятым решением — находка, даже если тесты зелёные. Но сначала спроси себя, не устарел ли док:
|
||
бывает, что решение сознательно поменяли, и тогда чинить надо документ.
|
||
|
||
## Как проверять
|
||
|
||
**Измеряй, а не рассуждай.** Когда вопрос звучит как «а хорошо ли раскладываются часы» или
|
||
«а сколько получается неполных семей» — не рассуждай о коде, напиши временный тест, который
|
||
печатает реальные числа, посмотри на них и удали его. Час размышлений о том, как поведёт себя
|
||
алгоритм, стоит дороже и ошибается чаще, чем один прогон, который печатает распределение.
|
||
|
||
**Проверяй подозрение до того, как о нём докладывать.** Половина находок «на глаз» рассыпается
|
||
при первом же взгляде на соседний файл: параметр, который считался забытым, передаётся из
|
||
вызывающего кода; поле, которого якобы нет в размере кадра, там есть. Проверенное подозрение —
|
||
находка, непроверенное — шум, который пользователю придётся разбирать за тебя.
|
||
|
||
## Что чинить
|
||
|
||
- **Недостающие тесты дописывай** в проект, который назначен политикой из `AGENTS.md`
|
||
(«Testing policy»). Тест генерации людей не место в тестах хоста.
|
||
- **Чини причину, а не симптом.** Падающий тест не ослабляют и не удаляют ради зелёного прогона.
|
||
Если тест действительно неправ — так и напиши в отчёте, с обоснованием, и меняй его осознанно.
|
||
- **Держись границ среза.** Найденное за его пределами — в отчёт строкой «замечено рядом», а не
|
||
в диф. Ревью, которое походя переписало соседний проект, невозможно посмотреть глазами.
|
||
- **Молча не меняй** версию протокола, форму сейва и публичное поведение API. Это отдельное
|
||
решение пользователя, даже когда оно очевидно правильное.
|
||
|
||
## Какие тесты гонять
|
||
|
||
Сначала читай код и имена тестов — ревью это сверка обещаний, не ритуал прогона. Когда
|
||
дописал недостающий тест или починил найденное, гоняй **только проект среза** из Testing
|
||
policy в `AGENTS.md`, с `--filter` на новый класс. Клиентский Vitest — только если ревью
|
||
трогает `src/HSchool.Client`. `HSchool.AppHost.Tests` — только если срез про HTTP, сокет
|
||
или хост. Весь solution, парный `npm test` «для уверенности» и прогон в начале ревью —
|
||
не делать.
|
||
|
||
Чего ждать и что делать:
|
||
|
||
- **`MSB3021` / `MSB3027`, «блокирует этот файл»** — у пользователя запущено приложение, оно
|
||
держит DLL. **Процесс не убивать.** Проверь то, что можно проверить без сборки .NET
|
||
(клиентские тесты, чтение кода, дизайн-доки), а в отчёте скажи прямо: .NET-часть не
|
||
прогонялась, потому что запущено приложение. В журнале такой срез — «проверен частично».
|
||
- **Концы строк.** В репозитории есть файлы и с CRLF, и с LF. Правка не должна переворачивать
|
||
файл целиком: сверься с `git diff --stat` — внезапные «изменено 400 строк» в файле, где ты
|
||
правил три, это оно.
|
||
- **Dev-сервер не запускать.** Если нужно посмотреть на UI, пользуйся уже запущенным
|
||
приложением пользователя; свой не поднимай и чужой не останавливай.
|
||
|
||
## Отчёт
|
||
|
||
Коротко и по делу, в конце каждого среза:
|
||
|
||
- что проверено и чем это подтверждено;
|
||
- что дописано (тесты — списком, по одной строке);
|
||
- что исправлено и почему это была ошибка;
|
||
- что осталось под вопросом — с формулировкой, по которой можно принять решение;
|
||
- какой срез следующий в очереди.
|
||
|
||
Пустой отчёт — тоже результат: «срез 2 проверен, все восемь тестов из фазы 6 на месте,
|
||
расхождений с дизайном нет» стоит написать явно. Это ровно та информация, ради которой
|
||
затевался журнал.
|