Инженерия ПО

Code Review: best practices

PR открыт. 800 строк diff. Ревьюер смотрит 10 минут, пишет «LGTM» и апрувит. Через неделю в проде баг - именно в этом PR. Кто виноват? Ревью существует именно для того, чтобы второй человек поймал то, что не заметил первый - но только если делать его правильно.

**Google** публично выпустили Engineering Practices - руководство по ревью, которому следуют тысячи команд
**Stripe** требует чтобы ревьюер мог объяснить изменение коллеге - если не может, PR не мерджится
**NASA JPL** используют формальные инспекции кода для mission-critical систем: один баг = потеря аппарата

Чеклист ревью

PR открыт, diff перед глазами - с чего начать? Без структуры ревьюер проверяет то, что попалось на глаза: отступы, имена переменных, синтаксический сахар. Важное - логические ошибки, race conditions, проблемы безопасности - остаётся незамеченным. **Чеклист** фиксирует приоритеты и делает ревью воспроизводимым.

Хороший чеклист движется от критического к несущественному. Первым идёт корректность: делает ли код то, что описано в задаче? Потом безопасность, производительность, тесты, читаемость - и только в конце стиль.

**Правило одного PR:** один PR - одно логическое изменение. PR размером >500 строк ревьюируется формально - никто не держит в голове весь контекст. Разбивайте большие задачи на серию мелких PR.

Какой аспект кода должен проверяться в ревью первым?

Конструктивная обратная связь

Технически правильный комментарий в ревью может разрушить отношения в команде. «Это неправильно» и «Здесь есть потенциальная проблема с X, потому что Y» - это одна и та же информация, но принимается совершенно по-разному. Форма подачи обратной связи определяет, будет ли ревью воспринято как помощь или атака.

Полезный фреймворк: **комментарий = наблюдение + обоснование + предложение**. Не «плохое имя», а «функция называется `process`, но выполняет только валидацию - `validate` точнее отражает ответственность». Разница: первое звучит как оценка автора, второе - как наблюдение о коде.

**Nit-комментарии:** мелкие стилистические замечания помечайте `nit:` - это сигнал автору, что блокировать мердж из-за этого не будут. `nit: можно написать короче через Array.from()`. Без такой маркировки автор не знает, насколько серьёзно замечание.

Комментарий «Это неэффективно» - в чём его главная проблема?

Автоматизация: lint, static analysis

Ревьюер - дорогой ресурс. Если он тратит 20% времени на замечания о форматировании, отступах и неиспользуемых импортах, команда теряет деньги. Всё, что можно формализовать правилом - должна проверять машина, не человек.

Автоматизация делится на два класса: **линтеры** (стиль, синтаксис, простые antipatterns) и **static analysis** (типы, потоки данных, потенциальные ошибки). В современном TypeScript-проекте эта связка закрывает 60-70% всех типичных замечаний в ревью.

**Правило:** если замечание можно выразить автоматическим правилом - добавьте правило в конфиг линтера. Иначе одно и то же замечание будет повторяться в каждом PR бесконечно.

Ревьюер регулярно оставляет комментарий «используйте === вместо ==». Лучшее решение:

Культура ревью в команде

Ревью - это не контроль качества одним человеком над другим. Это **совместное владение кодом**: после мерджа весь код принадлежит команде, а значит каждый заинтересован в том, чтобы понять его до мерджа. Культура ревью определяется тем, как команда воспринимает этот процесс.

Несколько принципов, которые разделяют токсичное ревью от здорового. **Код != автор**: замечания о коде не замечания о человеке. **Ревьюер учится тоже**: задавать вопросы «почему так?» - нормально, это не нападение. **Апрув - не формальность**: апрувить PR только потому что «надо разблокировать коллегу» разрушает смысл процесса.

**Google Engineering Practices** (публично доступны) - отличный ориентир. Ключевой принцип: ревьюер должен апрувить PR, если он **улучшает систему**, даже если код не идеален. Перфекционизм убивает скорость команды.

Code Review - это контроль качества: старший проверяет работу младшего.

Code Review - это совместное владение кодом: вся команда понимает каждое изменение до мерджа.

После мерджа поддерживать код будет вся команда. Ревью - это передача знаний, а не экзамен.

PR коллеги решает задачу, но архитектура не идеальна. Ревьюер не апрувит и требует полного рефакторинга. Это: