Code Review

Процесс проверки кода: как проводить и принимать review, оставлять конструктивные комментарии

Зачем нужно

  • Находить баги до merge в основную ветку
  • Поддерживать качество и единообразие кода
  • Делиться знаниями в команде
  • Снижать bus factor (зависимость от одного разработчика)

Где используется

  • В каждом Pull Request перед merge
  • В open-source проектах
  • В корпоративной разработке как обязательный этап

Предпосылки

Процесс Code Review

Workflow

1. Автор создаёт PR
2. Назначает reviewers
3. Reviewers читают код и оставляют комментарии
4. Автор исправляет замечания
5. Reviewers одобряют (Approve)
6. PR мержится

Роли

Роль Действия
Author Создаёт PR, отвечает на комментарии, вносит правки
Reviewer Читает код, оставляет комментарии, approve/request changes
Maintainer Финальное решение о merge

Как проводить review

1. Понять контекст

  • Прочитать описание PR
  • Понять задачу (issue/ticket)
  • Посмотреть на scope изменений

2. Проверить код

На что обращать внимание:

  • Корректность — решает ли код поставленную задачу?
  • Баги — есть ли edge cases, race conditions, null checks?
  • Читаемость — понятен ли код без объяснений?
  • Архитектура — правильно ли выбрана структура?
  • Производительность — нет ли лишних запросов, циклов?
  • Безопасность — нет ли уязвимостей (XSS, SQL injection)?
  • Тесты — покрыт ли новый код тестами?
  • Стиль — соответствует ли code style проекта?

3. Виды комментариев

Вопрос:

Почему здесь используется setTimeout вместо requestAnimationFrame?

Suggestion (предложение кода):

const users = data.filter(user => user.isActive);

GitHub превратит это в кнопку "Apply suggestion".

Замечание (nit — мелочь):

nit: переименовать `d` в `data` для читаемости

Блокирующее замечание:

Здесь потенциальная XSS-уязвимость. Нужно санитизировать
пользовательский ввод перед вставкой в DOM.

Похвала:

Отличное решение с кэшированием! Очень чисто.

Итог Review

Approve

Код готов к merge. Замечания незначительные или их нет.

Request Changes

Есть блокирующие проблемы, которые нужно исправить:

  • Баги
  • Проблемы безопасности
  • Серьёзные архитектурные проблемы

Comment

Замечания есть, но решение о merge оставляю автору.

Как принимать review (для автора)

Отвечать на комментарии

✓ "Хорошее замечание, исправил в коммите abc1234"
✓ "Обсудим: я выбрал этот подход потому что..."
✗ "Не согласен" (без объяснения)
✗ Игнорировать комментарии

Не воспринимать лично

  • Review направлен на код, не на вас
  • Замечания помогают расти
  • Несогласие — нормально, обсуждайте аргументированно

Resolve conversations

После исправления или обсуждения — нажимайте "Resolve conversation".

Best Practices

Для Reviewer

  1. Будьте конструктивны — предлагайте решения, не только критикуйте
  2. Объясняйте "зачем" — не просто "переименуй", а "переименуй для единообразия с..."
  3. Разделяйте блокеры и nit-ы — помечайте nit: для мелочей
  4. Хвалите хороший код — мотивация важна
  5. Отвечайте быстро — долгий review блокирует работу (целевое время: < 24ч)
  6. Не придирайтесь к стилю — для этого есть линтеры

Для Автора

  1. Маленькие PR — до 400 строк, легче ревьюить
  2. Хорошее описание — контекст экономит время reviewer
  3. Self-review — просмотрите свой diff перед запросом review
  4. Отвечайте на все комментарии — даже если просто "Сделано"
  5. Не спорьте бесконечно — если мнения разошлись, привлеките третье мнение

Настройка Branch Protection

В Settings → Branches → Branch protection rules:

  • Require pull request reviews before merging — обязательный review
  • Required number of reviewers: 1-2
  • Dismiss stale reviews — отзывать approve при новых коммитах
  • Require review from Code Owners — нужен approve от ответственных
  • Require status checks — CI должен пройти

Code Owners

Файл CODEOWNERS в корне репозитория:

# Глобальные владельцы
* @team-lead

# Фронтенд
/src/components/ @frontend-team
*.css @designer

# Бэкенд
/src/api/ @backend-team
/src/database/ @dba

Частые ошибки

  • Ревью за 30 секунд — "Looks good to me" без реального чтения
  • Чрезмерная придирчивость — блокировать PR из-за именования переменной
  • Не ревьюить тесты — в тестах тоже бывают баги
  • Задерживать review — блокируется работа автора
  • Переписывание чужого стиля — если код рабочий и читаемый, не нужно навязывать свой стиль

Практика

  1. Создайте PR и попросите коллегу (или друга) сделать review
  2. Оставьте 3 вида комментариев: вопрос, suggestion, nit
  3. Используйте suggestion-блоки GitHub
  4. Настройте branch protection для вашего репозитория
  5. Проведите self-review своего последнего PR

Связанные темы

Ресурсы