go-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, остальное по усмотрению.

Порядок ревью

1. Прочитай текст задачи (из YouTrack или описание ветки/MR)
2. Получи git diff текущих изменений
3. Проведи ревью каждой персоной

---

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 с указанием блокеров.