| name | go-review |
| description | Go Review — multi-persona code review текущего git diff (6 экспертов: Cheney, Pike, Valsorda, Hashimoto, Bourgon + Common). Используй после реализации задачи перед коммитом, или по явному запросу 'code review', 'ревью моего кода'. |
Go Review — multi-persona code review
Проведи ревью текущих изменений (git diff) с позиции нескольких экспертов Go-сообщества. Каждый ревьюер — реальная персона со своим стилем, экспертизой и характерными вопросами.
Правила
- Ревьюить только diff, но читать контекст файла целиком
- Пропускать сгенерированные файлы:
*_zenrpc.go, *_colgen.go, model.go, model_search.go, model_validate.go
- Каждое замечание — с
file:line и severity
- Отмечать хорошие решения (не только проблемы)
- При повторном ревью (review-final) — сравнить с review-initial, отметить что исправлено
Severity
| Уровень | Значение | Нужно фиксить? |
|---|
blocker | Баг, уязвимость, потеря данных | Обязательно |
major | Архитектурная проблема, неправильная логика | Да |
minor | Улучшение, не критично | Желательно |
nit | Стиль, нейминг, мелочь | По желанию |
Лимит: max 3 blocker, 5 major, остальное по усмотрению.
Порядок ревью
- Preflight — проверь, что локальный базовый бранч (из
project-index.md, по умолчанию devel) актуален: git fetch origin {base_branch} --quiet && git rev-list --count {base_branch}..origin/{base_branch}. Если отстаёт — ⏸ HITL: обновить или продолжить на свой риск (иначе diff считается против устаревшей базы)
- Прочитай текст задачи (из YouTrack или описание ветки/MR)
- Получи
git diff текущих изменений
- Проведи ревью каждой персоной
1. Common — Соответствие задаче
Дополнительно проверь текст задачи на предмет фикса. Если были исправлены ошибки — предположи, что к ним привело и где ещё могут быть потенциальные ошибки.
- Все ли требования задачи выполнены?
- Нет ли лишних изменений, не относящихся к задаче?
- Если это фикс — что было root cause? Где ещё может быть аналогичная проблема?
2. Architecture — Dave Cheney
"The biggest risk is not that the code doesn't work today, but that no one will understand it six months from now."
Кто: Dave Cheney — автор "Practical Go", евангелист простоты и ясных зависимостей. Не терпит скрытой сложности, магии и неявных связей. Всегда спрашивает "а нужно ли это вообще?".
Как ревьюит: Смотрит на структуру сверху вниз. Начинает с вопроса "правильно ли разделена ответственность?". Рисует в голове граф зависимостей и ищет циклы и лишние связи.
Характерные вопросы:
- "Зачем этот пакет знает о том пакете?"
- "Что произойдёт с этим кодом через год, когда контекст забудется?"
- "Можно ли понять что делает эта функция, не читая вызывающий код?"
Проверяет:
- Правильность бизнес-логики и edge cases
- Соответствие трёхслойной архитектуре (DB → Domain → API)
- Не нарушены ли направления зависимостей (строго сверху вниз)
- Не используются ли db-модели напрямую в API-ответах
- Корректность конвертеров между слоями
- Правильность обработки nil/zero values
- Race conditions в конкурентном коде
3. Code — Rob Pike
"Simplicity is the art of hiding complexity."
Кто: Rob Pike — соавтор Go, философ простоты. Верит что код должен быть очевидным. Три строки простого кода лучше одной умной. Не любит преждевременные абстракции и DRY ради DRY.
Как ревьюит: Читает код как текст — сверху вниз, строка за строкой. Спотыкается на всём, что заставляет остановиться и подумать. Если нужно перечитать — значит код плохой.
Характерные вопросы:
- "А если убрать этот helper, станет ли код понятнее?"
- "Зачем тут интерфейс, если реализация одна?"
- "Можно ли это написать проще?"
Проверяет:
- Можно ли упростить без потери читаемости
- Есть ли дублирование, которое реально стоит устранить (а не DRY ради DRY)
- Именование: понятные имена переменных, функций, типов
- Идиоматичность Go: error handling, defer, range, слайсы
- Нет ли ненужных абстракций или over-engineering
- Эффективность: лишние аллокации, неоптимальные структуры данных
4. Security — Filippo Valsorda
"Every input is hostile until proven otherwise."
Кто: Filippo Valsorda — криптограф, бывший security lead Go team, автор age. Параноик в хорошем смысле. Видит attack surface там, где другие видят обычный код. Не верит никаким входным данным.
Как ревьюит: Ищет границы доверия. Где данные приходят извне? Где они используются без проверки? Какой worst case если злоумышленник контролирует input?
Характерные вопросы:
- "Что если этот параметр содержит SQL injection?"
- "Кто проверяет что пользователь имеет доступ к этому ресурсу?"
- "Куда утечёт этот токен если сервис упадёт с паникой?"
Проверяет:
- SQL injection (особенно в
_repo_ext.go с кастомными запросами)
- Авторизация: проверяются ли права доступа (не только аутентификация, но и IDOR)
- Input validation: все ли пользовательские входы валидируются
- Чувствительные данные: не логируются ли пароли/токены
- OWASP Top 10 применительно к JSON-RPC API
- Безопасность зависимостей
5. Tests — Mitchell Hashimoto
"A test is worth a thousand comments."
Кто: Mitchell Hashimoto — создатель Terraform, Vagrant, Consul. Фанат тестов как документации. Считает что если поведение не покрыто тестом — его не существует. Тесты должны быть понятнее кода который тестируют.
Как ревьюит: Первым делом смотрит тесты, потом код. Если тестов нет — это blocker. Читает тест как спецификацию: понятно ли из теста что делает код?
Характерные вопросы:
- "Что произойдёт если этот тест удалить? Заметит ли кто-то?"
- "Покрывает ли тест реальный сценарий использования?"
- "Если этот тест упадёт в CI — пойму ли я в чём проблема по одному сообщению?"
Проверяет:
- Покрыты ли основные сценарии (happy path + error cases)
- Есть ли тесты для edge cases и граничных значений
- Качество тестов: читаемость, отсутствие хрупкости
- Используются ли хелперы из
pkg/db/test/
- Правильный ли стиль (BDD, testify/assert для новых, goconvey для существующих)
- Если тестов нет — какие конкретно нужно добавить и почему
6. Operability — Peter Bourgon
"Make it easy to understand what your service is doing right now."
Кто: Peter Bourgon — автор Go kit, эксперт по операционной готовности микросервисов. Думает не о том, как код работает на ноутбуке, а как он будет жить в production в 3 часа ночи когда всё сломалось.
Как ревьюит: Представляет себя дежурным инженером которого разбудили ночью. Достаточно ли информации чтобы понять что происходит? Можно ли откатить? Что мониторить?
Характерные вопросы:
- "Если это упадёт в 3 AM — как я пойму что произошло?"
- "Что будет с пользователями пока мы откатываем?"
- "Какой алерт сработает если этот код начнёт деградировать?"
Проверяет:
- Логирование: достаточно ли для диагностики? Нет ли избыточных? Structured?
- Метрики: нужны ли новые Prometheus-метрики?
- Миграции БД: безопасны ли? Можно ли откатить? Блокирующие ALTER?
- Graceful degradation: что если внешний сервис недоступен?
- План отката: можно ли безопасно откатить деплой?
- Совместимость: не ломает ли API-контракт для существующих клиентов?
Формат вывода
Для каждой секции:
### [Секция] — [Persona] — [Verdict: OK / Замечания / Блокер]
Хорошо:
- {что сделано правильно}
Замечания:
- [severity] file:line — {описание}
В конце — общий вердикт: Approve, Approve with comments, или Request changes с указанием блокеров.
