Как проводилось code review?

Подход к проведению Code Review

В нашей команде Code Review был не формальной процедурой, а ключевым инструментом повышения качества кода, знаний команды и снижения долгосрочных рисков. Мы использовали Git Flow с ветками feature/, hotfix/ и release/, а процесс ревью был интегрирован в наш CI/CD-пайплайн в GitLab (аналогично в GitHub Actions или Bitbucket).

Основные этапы процесса

1. Предварительные требования перед созданием Merge Request (MR) / Pull Request (PR):

    *   Код должен быть покрыт **юнит-тестами** и **интеграционными тестами** (мы использовали PHPUnit и иногда Codeception).
    *   Все существующие тесты должны проходить (`phpunit`).
    *   Код должен проходить статический анализ без критических ошибок. Мы использовали **PHPStan** (уровень не ниже 6) и **Psalm** для сложных проектов.
    *   Код должен соответствовать **PSR-12** (проверялось через **PHP-CS-Fixer**). Форматирование запускалось автоматически в pre-commit хуке или в пайплайне.
    *   Должна быть обновлена документация (phpDoc для публичных методов, обновление README если меняется API).

1. Создание и описание MR:

    *   Разработчик создает MR и заполняет шаблон, который включает:
        *   **Цель изменений** (какую задачу из Jira/Tracker решает).
        *   **Тип изменений** (новый функционал, рефакторинг, исправление бага).
        *   **Список ключевых изменений** (какие файлы/классы затронуты).
        *   **Чек-лист** (тесты добавлены, документация обновлена и т.д.).
        *   **Скриншоты / примеры работы** — если изменения касаются UI или API.

    Пример нашего шаблона в GitLab:

```markdown
## Что сделано?
[Краткое описание]

## Зачем это нужно?
[Ссылка на задачу, бизнес-логика]

## Как это было протестировано?
- [ ] Добавлены юнит-тесты
- [ ] Проверено вручную на staging
- [ ] Прошел статический анализ (PHPStan)

## Потенциальные риски
[Что может сломаться?]

## Скриншоты (если применимо)
[Вставка изображений]
```

3. Процесс ревью:

    *   **Автоматические проверки (CI):** Первым делом запускался пайплайн. Если он не проходил, ревьюер даже не приступал к просмотру кода.
    *   **Назначение ревьюеров:** Обязательно 1-2 основных ревьюера из команды (ротация). Плюс, при изменениях в конкретном модуле, привлекался его **владелец (code owner)**.
    *   **Фокус ревью:** Мы смотрели не только на синтаксис, но на:
        *   **Архитектурную целостность:** Соответствует ли изменение общим принципам проекта (DDD, слоистая архитектура, etc.).
        *   **Читаемость и ясность:** Понятны ли имена переменных, не слишком ли сложные методы.
        *   **Отсутствие дублирования (DRY):** Не появился ли похожий код в другом месте.
        *   **Эффективность алгоритмов:** Особенно в критичных по производительности местах.
        *   **Обработка ошибок и edge-кейсы:** Все ли исключительные ситуации учтены.
        *   **Безопасность:** Нет ли потенциальных SQL-yязвимостей, XSS (в шаблонах), проблем с авторизацией.
    *   **Коммуникация:** Все комментарии оставлялись **вежливо** и конструктивно, с предложениями по улучшению. Использовались **suggestions** в GitLab/GitHub, которые автор мог применить одной кнопкой.

    ```php
    // Пример комментария ревьюера с suggestion:
    // Было в коде:
    if ($user->isActive() == true) {
    
    // Ревьюер: "Упрости условие, достаточно `if ($user->isActive())`"
    // После применения suggestion:
    if ($user->isActive()) {
    ```

    *   **Обсуждение:** Если возникали архитектурные споры, мы проводили короткие **ad-hoc обсуждения** у монитора или созванивались, чтобы не затягивать MR.

1. Фиксация результатов:

    *   Автор вносил правки, пушил в ту же ветку — все комментарии отмечались как решенные.
    *   После **approve** от всех назначенных ревьюеров и **прохождения всех CI1-чеков** MR разрешался к мержу.
    *   **Мерж выполнял автор MR**, чтобы он взял на себя финальную ответственность. Мы использовали **squash merge** для чистоты истории, либо **rebase merge** для важных фичей.

Ключевые принципы, которых мы придерживались

• Скорость — важно: MR не должен висеть днями. Мы стремились дать фидбек в течение 4-8 рабочих часов.
• Небольшой размер MR: Идеальный MR — это 200-400 строк изменений. Большие изменения разбивались на логические части.
• Ревью — не тестирование: Мы ожидали, что код уже протестирован автором. Ревью фокусируется на качестве, а не на поиске багов, которые ловятся тестами.
• Обучение через ревью: Для джунов это был лучший способ учиться у сеньоров. Для всех — возможность быть в курсе изменений в разных частях системы.
• Инструментарий: Помимо встроенных возможностей GitLab, мы иногда использовали PhpStorm с плагином для просмотра MR прямо в IDE, что очень удобно для навигации по коду.

Такой системный подход превращал code review из рутины в мощный инженерный практикум, который значительно повышал надежность, поддерживаемость кодовой базы и скорость разработки в долгосрочной перспективе.