# Code review

_Совместная работа · GitLab Knowledge Base_

**TL;DR:** Чтение и обсуждение кода другого разработчика перед merge. Главная
цель - поделиться контекстом, а не найти баги (баги - побочный
результат). Хороший ревью улучшает код и распространяет знание о
системе по команде.


Code review - практика чтения изменений перед влитием в основную
ветку. На уровне инструмента - комментарии в Pull Request (см.
[pull-request](/courses/git/kb/pull-request.md)). На уровне культуры - отдельный навык, влияющий
на скорость команды больше, чем выбор фреймворка.

## Зачем

Главные функции по убыванию:

1. **Распространение знания.** Через год ревьюер всё ещё знает, что
   в этом куске кода происходит. Без ревью - только автор. Команда
   устойчивее к увольнениям и отпускам.
2. **Договорённость о стандарте.** Каждое ревью - это разговор «как
   мы пишем тут». Стандарт держится живым.
3. **Поиск проблем.** Логические ошибки, неучтённые edge case,
   проблемы производительности, безопасность. Автор замылен - ревьюер
   видит свежим взглядом.
4. **Защита от регрессий.** Старший ревьюер ловит «эта правка
   сломает другую систему».
5. **Менторство.** Обратная связь - главный способ роста новичка.

«Ловить баги» - пятое в списке. Большинство багов найдут тесты;
ревью отлавливает то, что тесты не проверяют (архитектурные решения,
читаемость, неявные допущения).

## Хорошие практики автора

- **Маленькие PR'ы.** До 300 строк, в идеале - до 100. Ревьюер
  прочитает быстро, найдёт больше.
- **Описание PR.** Зачем, что, как проверить. Без описания ревьюер
  тратит первые 10 минут на восстановление контекста.
- **Скриншоты для UI.** Сэкономит ревьюеру локальный запуск.
- **Self-review перед запросом ревью.** Прочитай свой diff,
  исправь очевидное - никто другой не должен находить за тебя
  `console.log` или TODO.
  - **Не обижайся на комментарии.** Ревью - про код, не про автора.
  Если комментарий звучит обидно - обычно это привычка ревьюера, не
  злой умысел. Спроси «что ты предлагаешь?», не возражай в защиту.

## Хорошие практики ревьюера

- **Отвечай быстро.** PR > 24 часов без ответа - авторе теряет
  контекст. Идеал - несколько часов.
- **Объясняй, не приказывай.** Не «исправь», а «здесь возможна
  проблема X - что думаешь?». Особенно с младшими.
- **Различай уровни замечаний.** Использовать префиксы:
- `nit:` или `(non-blocking)` - мелочь, можно проигнорировать.
- `suggestion:` - идея, не обязательная к применению.
- `must:` или `(blocking)` - нужно поправить до merge.
- `question:` - просто вопрос, без претензии.
- **Замечание = причина.** «Можно лучше» - плохо. «Здесь возможен
  null reference, см. строку X в auth.py» - хорошо.
  - **Approve, когда готово.** Не «approve с замечаниями» - это
  путаница. Если есть блокирующее - request changes. Если нет - 
  approve.

## Структура замечаний (стандарт Conventional Comments)

```
praise: Хорошо реализовано через композицию, чисто.

nit: Лучше переименовать `data` в `userResponse` - понятнее.

suggestion: Можно вынести в утилиту, использовать в trei местах.

question: Зачем тут try/catch без обработки?

issue (blocking): Этот код проглатывает ошибку - нужно либо
пробросить, либо логировать.
```

Префикс сразу говорит о тоне и обязательности. Сильно ускоряет
обсуждение.

## Что искать

Чек-лист «о чём думать при ревью»:

- **Понятность.** Через год другой разработчик прочитает и поймёт?
- **Корректность.** Решает ли задачу из описания PR?
- **Edge cases.** Что с null, пустым массивом, очень большими
  данными, гонкой?
- **Тесты.** Покрывают ли изменение? Тестируют ли поведение, а не
  реализацию?
- **Безопасность.** Пользовательский ввод санируется? Доступы
  проверяются?
- **Производительность.** Не появилось ли n² там, где должно быть n?
- **Согласованность.** Стиль соответствует остальному коду?

Не пытаться найти всё за один проход. Сначала understanding
(понимаю ли я, что происходит), потом correctness, потом мелочи.

## Подводные камни

- **Bikeshedding.** Бесконечные обсуждения цвета кнопки/имени
  переменной. Договорись с командой о time-box или линтер-правилах.
- **Approve «не глядя».** На больших PR это норма, но это значит,
  что ревью не работает. Решение - заставлять разбивать PR.
- **Конфликт уровней.** Senior ревьюит junior'а слишком жёстко,
  junior ревьюит senior'а слишком робко. Договоритесь, что в ревью
  нет иерархии - оба отвечают за качество.
- **Эго-ревью.** «Я бы написал по-другому». Если код решает задачу
  и читается - это легитимный вариант, не повод требовать переписать.


## См. также

- [Pull Request (PR)](/courses/git/kb/pull-request.md)
- [Стратегии merge PR (GitHub)](/courses/git/kb/merge-strategies.md)
- [Атомарный коммит](/courses/git/kb/atomic-commit.md)