Код-ревью, командные стандарты и Definition of Done

Весь трек до этого места говорил про код: какие у него должны быть имена, границы, зависимости, тесты, обработка ошибок. Всё это — знания, живущие в голове отдельного инженера. Но код пишут не инженеры, а команды, и между «я знаю, как надо» и «в нашем репозитории так и есть» лежит пропасть, которую не закрыть ни одной книжкой по чистому коду.

Эта статья — про мост через эту пропасть. Про три механизма, которые превращают личное знание в свойство системы:

1. Код-ревью — точка, где принципы встречаются с реальным изменением до того, как оно попадёт в main.
2. Командные стандарты — записанные решения, которые не нужно принимать заново на каждом PR.
3. Definition of Done — общее определение слова «готово», без которого команда спорит не о коде, а о том, закончена ли работа.

Все три легко выродиться в бюрократию. Ровно так же, как SOLID вырождается в карго-культ (см. обзор трека), ревью вырождается в ритуал «два аппрува перед мержем», а DoD — в чеклист, который никто не читает. Поэтому по каждому механизму разбираем не только «как правильно», но и зачем, чем платим и как выглядит вырождение.

1. Зачем на самом деле нужно ревью

Спросите инженера, зачем нужно код-ревью, — почти наверняка услышите «чтобы ловить баги». Это неточный ответ, и неточность дорого стоит: если считать поиск багов главной целью, все решения о процессе будут приниматься неправильно.

Самое цитируемое эмпирическое исследование на эту тему — Expectations, Outcomes, and Challenges of Modern Code Review (Bacchelli, Bird, ICSE 2013). Авторы опросили и понаблюдали сотни разработчиков в Microsoft, сравнив то, чего люди ожидают от ревью, с тем, что реально происходит. Результат: поиск дефектов стоит на первом месте в ожиданиях и далеко не на первом — в фактических результатах. Реально ревью производит:

• распространение знания о коде — второй человек узнаёт, что происходит в этой части системы;
• улучшение читаемости и дизайна — большинство комментариев не про баги, а про понятность, именование, структуру, альтернативные решения;
• обучение — джуниоры учатся у сеньоров, сеньоры узнают домен от тех, кто в нём глубже;
• владение кодом становится коллективным — снижается bus factor;
• соблюдение стандартов — новичок узнаёт «как здесь принято» не из вики, а из конкретного комментария к своей строке.

Похожие выводы у Google: в Modern Code Review: A Case Study at Google (Sadowski и др., ICSE-SEIP 2018) первичной мотивацией названо не обнаружение дефектов, а поддержание понятности и сопровождаемости кода, при этом медианный размер изменения крайне мал, а медианная задержка до первого ответа измеряется часами, а не днями.

Отсюда практический вывод, на котором держится вся статья:

Ревью — это не контроль качества, а канал коммуникации. Дефекты, которые можно поймать машиной, должна ловить машина. Человеку остаётся то, чего машина не умеет: понять смысл изменения и его последствия через год.

Именно поэтому ревью стоит в цепочке барьеров далеко не первым.

Каждый барьер левее — дешевле на порядок. Если на ревью вы обсуждаете расстановку пробелов, вы сожгли самый дорогой ресурс процесса на задачу, которую решает gofmt за 5 миллисекунд.

Немного истории: откуда это всё

Формальные инспекции Фагана давали отличную полноту обнаружения дефектов, но стоили десятки человеко-часов на тысячу строк — на такой скорости современный продукт не выпустить. Современное ревью (термин modern code review закрепили Rigby и Bird в работе Convergent Contemporary Software Peer Review Practices, FSE 2013) — это осознанный размен: меньше формальности и полноты, зато асинхронно, быстро, на каждое изменение и с широким охватом.

2. Экономика ревью: почему размер PR — главный рычаг

Ревью — очередь. Значит, к нему применима теорема Литтла: L = λ · W, где L — среднее число PR «в работе», λ — темп поступления, W — среднее время прохождения. Из неё следует то, что видно на глаз в любой команде: если ревьюеры отвечают раз в сутки, а разработчиков десять, у вас всегда висит гора незамерженных веток, каждая из которых стареет и конфликтует с main.

Второй фундаментальный фактор — размер изменения. Данные Cisco/SmartBear (описаны в Best Practices for Peer Code Review) дают устойчивые ориентиры: эффективность обнаружения дефектов резко падает при скорости просмотра выше ~500 строк в час и при объёме сессии больше ~400 строк или 60 минут. Google в своём руководстве Small CLs прямо называет размер изменения главным фактором скорости и качества.

Интуиция за формой кривых:

• Внимание — расходуемый ресурс. На первых 200 строках ревьюер думает; на 900-й он листает.
• Большой PR нельзя откатить по частям. Замечание «здесь неверная модель» означает переделку недели работы — социальное давление толкает к «ладно, потом поправим».
• Большой PR долго висит → конфликты с main → rebase → ревьюер должен смотреть заново → висит ещё дольше. Петля положительной обратной связи.

Как физически делать PR маленькими

Основная отговорка — «изменение неделимо». Почти всегда делимо, просто требует техники:

Последняя строка важнее, чем кажется: смешивание автогенерируемого шума (обновление lock-файла, прогон форматтера, регенерация protobuf) с осмысленным кодом — самый частый способ сделать 60-строчное изменение нечитаемым.

3. Жизненный цикл изменения

Прежде чем спорить о деталях, полезно увидеть процесс целиком как конечный автомат. Именно он неявно зашит в GitHub/GitLab/Gerrit, и большинство «проблем с ревью» — это проблемы переходов, а не состояний.

Два перехода, которые команды чаще всего теряют:

• Черновик → ГотовКРевью через self-review. Автор обязан первым прочитать собственный диф в интерфейсе ревью — не в IDE. Это ловит забытые console.log, закомментированный код, случайно попавшие файлы и просто «ой, тут я неправильно назвал». Стоит 5 минут, экономит целый круг асинхронной переписки, который стоит часы календарного времени.
• ТребуетПравок → Закрыт. PR можно и нужно закрывать. Если в процессе обсуждения выяснилось, что подход неверен, закрытие — не поражение автора, а самый дешёвый из возможных исходов: неверное решение остановлено до мержа.

Протокол во времени

Автомат не показывает главного — задержек. Вот та же история как последовательность взаимодействий, где видно, где утекает календарное время.

Ключевая асимметрия: ревьюер должен отвечать быстро, но не обязан отвечать полно за один заход. Google формулирует это в Speed of Code Reviews: реагировать в течение одного рабочего дня, при этом допустимо и полезно прислать частичный набор комментариев, а не копить идеальный полный разбор. Скорость первого отклика важнее полноты, потому что автор всё это время заблокирован или, что хуже, переключился на другую задачу и начал накапливать WIP.

4. Что смотреть на ревью: слои внимания

Плохое ревью хаотично: ревьюер идёт по дифу сверху вниз и комментирует то, что бросается в глаза (обычно — именование в первом файле). Хорошее ревью проходит несколько слоёв, от самого дорогого к самому дешёвому.

Порядок не случаен: если изменение проваливается на слое 1 или 2, комментировать слои 5–7 бессмысленно и даже вредно — вы тратите внимание автора на код, который будет выброшен. Отсюда правило: сначала прочитай PR целиком, потом комментируй.

Приоритизация замечаний: что вообще должен делать человек

Левый нижний квадрант в идеале никогда не появляется в комментариях: это работа prettier/gofmt/black/ruff --fix, подключённая через pre-commit и продублированная в CI. Каждый комментарий оттуда — сигнал, что в процессе дыра.

5. Разбор реального PR: как выглядит ревью по слоям

Абстракции полезны, но лучше один разобранный диф. Ниже — правдоподобное изменение: добавляем эндпойнт возврата средств.

# --- ПРИШЛО НА РЕВЬЮ ---------------------------------------------------
@app.post("/api/refund")
def refund():
    data = request.get_json()
    order = db.query(f"SELECT * FROM orders WHERE id = {data['order_id']}")  # (1)
    if order["status"] == "paid":                                            # (2)
        amount = data.get("amount", order["total"])                          # (3)
        resp = requests.post(                                                # (4)
            "https://api.payments.example/v1/refunds",
            json={"charge": order["charge_id"], "amount": amount},
        )
        db.execute(f"UPDATE orders SET status='refunded' WHERE id={order['id']}")  # (5)
        print("refunded", order["id"], amount)                               # (6)
        return {"ok": True}
    return {"ok": False}                                                     # (7)

Разбор по слоям внимания — в порядке убывания важности, а не в порядке строк:

Обратите внимание, чего в списке нет: замечаний про пробелы, про data как имя переменной, про «лучше f-строку заменить на .format()». Они бы утопили семь настоящих проблем в шуме.

Как выглядит результат после круга правок:

# --- ПОСЛЕ РЕВЬЮ -------------------------------------------------------
@app.post("/api/refund")
def refund_endpoint():
    """HTTP-слой: разбор запроса, коды ответа. Никакой бизнес-логики."""
    body = RefundRequest.model_validate(request.get_json())  # схема валидирует типы
    try:
        result = refund_service.refund(
            order_id=body.order_id,
            amount=body.amount,
            idempotency_key=request.headers["Idempotency-Key"],
        )
    except OrderNotFound:
        return {"error": "order_not_found"}, 404
    except RefundNotAllowed as e:
        # доменная ошибка -> 409: клиент может отличить её от сбоя
        return {"error": e.code}, 409
    return {"refund_id": result.id, "amount": str(result.amount)}, 200

class RefundService:
    """Домен: правила возврата. Не знает про HTTP и про фреймворк."""

    def refund(self, order_id: OrderId, amount: Decimal | None,
               idempotency_key: str) -> Refund:
        with self._uow.transaction() as tx:            # атомарность локального состояния
            order = tx.orders.get_for_update(order_id)  # блокировка строки от гонок
            if order is None:
                raise OrderNotFound(order_id)

            refundable = order.total - tx.refunds.sum_for(order_id)
            amount = amount if amount is not None else refundable
            if not (Decimal("0") < amount <= refundable):
                raise RefundNotAllowed(code="amount_exceeds_refundable")

            # 1) фиксируем намерение ДО внешнего вызова: если процесс упадёт,
            #    восстановитель увидит запись в статусе pending и доведёт её
            refund = tx.refunds.create(order_id, amount, idempotency_key,
                                       status=RefundStatus.PENDING)

        # 2) внешний вызов вне транзакции: не держим блокировку БД на сетевом IO
        try:
            provider_id = self._gateway.refund(
                charge_id=order.charge_id, amount=amount,
                idempotency_key=idempotency_key,  # повтор не спишет дважды
            )
        except GatewayTimeout:
            # состояние на стороне провайдера неизвестно — оставляем PENDING,
            # сверщик разберётся по idempotency_key
            log.warning("refund.gateway_timeout", extra={"refund_id": refund.id})
            raise

        with self._uow.transaction() as tx:
            tx.refunds.mark_succeeded(refund.id, provider_id)
        log.info("refund.succeeded", extra={"refund_id": refund.id,
                                            "order_id": order_id, "amount": str(amount)})
        metrics.refund_amount.observe(float(amount))
        return refund

Стоимость и сложность: обе версии — O(1) по числу запросов к БД на один возврат (плюс один агрегирующий SUM, индексируемый по order_id), но вторая делает три обращения к БД вместо двух и удерживает блокировку строки на время локальной транзакции, а не на время сетевого вызова. Это осознанный размен: чуть больше латентности и кода — в обмен на отсутствие класса «деньги ушли, а система думает, что нет». Такие размены и есть предмет ревью; ни один линтер их не увидит.

6. Как писать замечания: тон, формат, обязательность

Ревью — единственное место в инженерной работе, где систематически критикуют чужой труд в письменном виде и с сохранением истории. Игнорировать социальную сторону — значит получить процесс, который люди тихо саботируют.

Различайте блокирующее и необязательное

Самая ценная конвенция в индустрии — Conventional Comments: каждый комментарий начинается с метки и, при необходимости, декоратора.

praise: отличная идея вынести сверку в отдельный воркер — это снимает
        зависимость от того, доживёт ли запрос до ответа платёжки.

issue (blocking): у HTTP-клиента нет таймаута; при зависании провайдера
        выест пул воркеров. Нужен timeout= и ретраи с джиттером.

question: правильно ли я понимаю, что при частичном возврате остаток
        всё ещё можно вернуть отдельным запросом? Тогда нужен тест на это.

suggestion (non-blocking): здесь напрашивается `sum_for` вместо ручного цикла —
        на твоё усмотрение, мержить не блокирую.

nitpick (non-blocking): опечатка в тексте ошибки, "recieved" -> "received".

thought: если таких возвратов станет много, возможно, стоит вынести
        сверку в отдельный сервис. Не в этом PR, просто фиксирую мысль.

Что это даёт:

• Автор сразу видит, что чинить обязательно, а что — по желанию. Без меток любая фраза читается как требование, и PR разбухает.
• Ревьюер получает разрешение писать мысли вслух, не блокируя мерж. Ценные наблюдения перестают теряться.
• praise: — не вежливость ради вежливости: он документирует удачные решения, чтобы они распространялись.

Правила формулировок, которые реально работают

1. Критикуйте код, а не человека. «Эта функция делает три вещи» вместо «ты смешал три ответственности». Разница не в вежливости, а в предмете обсуждения.
2. Задавайте вопрос, если не уверены. «Что произойдёт, если сюда придёт пустой список?» лучше, чем «здесь баг» — в половине случаев автор ответит, что кейс невозможен, и вы узнаете про домен.
3. Объясняйте «почему». Замечание без обоснования не учит и порождает спор об авторитете. Ссылка на командный стандарт или статью закрывает вопрос за один круг.
4. Предлагайте, а не требуйте, когда речь о вкусе. Формулировка «предлагаю» и метка non-blocking дешевле долгой переписки.
5. Не переписывайте PR за автора. Если вы диктуете каждую строку — вы либо взяли не ту задачу для этого человека, либо вам нужно было писать этот код самому.
6. Приводите пример кода для сложного замечания. suggestion-блок с готовым кодом (GitHub умеет применять его в один клик) экономит круг переписки.
7. Не требуйте идеала. Формулировка Google в The Standard of Code Review: изменение нужно одобрять, когда оно определённо улучшает общее состояние системы, даже если не идеально. Иначе процесс превращается в тормоз, а совершенство — во врага улучшений.

Как автору отвечать

• Ответить нужно на каждый тред: «сделал», «сделал иначе, потому что…», «не согласен, потому что…». Молча закрытый тред — источник конфликтов.
• Не спорить о вкусовщине дольше двух реплик. Третья реплика — созвон на 5 минут или эскалация к командному стандарту.
• Правки присылать fixup-коммитами, а не force-push с переписанной историей: ревьюер должен видеть, что изменилось со времени его прочтения. Схлопнуть историю можно при мерже (git rebase --autosquash, squash-merge).
• Разногласие разрешает данные, стандарт или владелец кода — в таком порядке. Если ни того, ни другого нет, значит, у вас обнаружился недостающий стандарт: заведите его после мержа.

7. Командные стандарты: как перестать спорить об одном и том же

Стандарт — это записанное однажды принятое решение, чтобы не принимать его заново. Ценность стандарта не в том, что он «правильный», а в том, что он единый. Табы против пробелов не имеют объективно верного ответа; имеет значение только то, что в репозитории один вариант.

Признак отсутствующего стандарта: одно и то же обсуждение всплывает на третьем PR подряд. Это триггер — не выигрывать спор в очередной раз, а зафиксировать правило.

Уровни стандартов и где они живут

Главное правило: стандарт, который не проверяется автоматически, — это пожелание. Всё, что может быть выражено конфигом, должно быть выражено конфигом.

# .pre-commit-config.yaml — гейт на машине разработчика, до пуша.
# Тот же набор дублируется в CI: локальный хук можно пропустить через --no-verify,
# CI пропустить нельзя.
repos:
  - repo: https://github.com/astral-sh/ruff-pre-commit
    rev: v0.6.9
    hooks:
      - id: ruff            # линтер: неиспользуемое, shadowing, антипаттерны
        args: [--fix]
      - id: ruff-format     # форматтер: спор о стиле закрыт навсегда
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v5.0.0
    hooks:
      - id: trailing-whitespace
      - id: end-of-file-fixer
      - id: check-merge-conflict
      - id: detect-private-key      # секрет не должен доехать даже до ветки
  - repo: https://github.com/pre-commit/mirrors-mypy
    rev: v1.11.2
    hooks:
      - id: mypy
        args: [--strict]

# CODEOWNERS — кто обязателен на ревью. Побеждает ПОСЛЕДНЕЕ совпавшее правило.
# Документация: https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners
*                       @team/backend
/billing/               @team/payments @alice
/infra/                 @team/platform
/docs/                  @team/backend @tech-writers
*.sql                   @team/dba
/security/policies/     @team/appsec

CODEOWNERS решает главную проблему масштабирования: ревьюер выбирается не «кто свободен», а «кто отвечает за эту область». Побочный эффект — файл становится живой картой владения системой; если после инцидента вы не можете сказать, чей это код, начните с него.

PR-шаблон: чеклист, который не бесит

<!-- .github/pull_request_template.md -->
## Что и зачем
Ссылка на задачу: PAY-1423
Проблема, которую решаем (2–3 предложения, понятные через год):

## Как проверить
Шаги/команда/скриншот или лог:

## Влияние на прод
- [ ] Миграция БД: нет / есть — обратима, применяется до деплоя кода
- [ ] Флаг: `refunds_v2`, по умолчанию выключен
- [ ] Обратная совместимость API сохранена
- [ ] План отката: выключить флаг

## Ревьюеру
Особое внимание: логика частичных возвратов в `RefundService.refund`
Сознательно НЕ сделано в этом PR: сверка с провайдером (PAY-1440)

Обратите внимание: в шаблоне нет пунктов «код отформатирован», «тесты проходят», «нет закомментированного кода». Это проверяет CI. Каждый пункт-галочка, который может проверить машина, обучает команду ставить галочки не глядя — и тогда обесцениваются и остальные.

Style guide: не пишите свой с нуля

Возьмите готовый и допишите дельту. Реальные, поддерживаемые источники:

• Google Style Guides — C++, Java, Python, Go, TypeScript, Shell.
• Go Code Review Comments — по сути канонический чеклист ревью для Go; см. также трек Go.
• Effective Go, PEP 8, Rust API Guidelines.
• Google Engineering Practices — руководства и для ревьюера, и для автора изменения.

Свой документ имеет смысл держать коротким: только решения, которые отличаются от общепринятых, и только с обоснованием. Стайлгайд на 40 страниц никто не прочитает, а значит, он не стандарт.

ADR: стандарты уровня архитектуры

Решения вроде «почему мы выбрали Kafka, а не RabbitMQ» фиксируются в Architecture Decision Record — коротком файле в репозитории рядом с кодом:

# ADR-014: Идемпотентность операций с деньгами через Idempotency-Key

Статус: принято, 2026-03-11
Контекст: клиенты ретраят POST при таймаутах; повторное списание недопустимо.
Решение: все мутирующие эндпойнты биллинга требуют заголовок Idempotency-Key;
ключ и результат хранятся 24 часа в таблице idempotency_records.
Последствия: +1 запись в БД на операцию; клиенты обязаны генерировать UUID;
ревью PR в /billing проверяет наличие ключа (см. CODEOWNERS @team/payments).
Альтернативы: дедупликация по хешу тела — отвергнута, ломается при ретрае
с изменённым таймстемпом.

Формат предложил Michael Nygard, живой шаблон — adr.github.io. ADR превращает «так исторически сложилось» в «вот решение, вот контекст, вот когда его надо пересмотреть».

8. Definition of Done

DoD пришёл из Scrum и в Scrum Guide 2020 описан как обязательство, привязанное к инкременту: формальное описание состояния, при котором продукт удовлетворяет требуемому качеству. Работа, не удовлетворяющая DoD, не может быть предъявлена как результат.

Практически DoD решает одну проблему: слово «готово» без него означает разные вещи для разных людей. Разработчик: «код написан». Тестировщик: «проверено». Продакт: «пользователи видят». Пока эти определения не совпадают, команда будет систематически недооценивать задачи ровно на разницу между ними.

DoR, DoD и критерии приёмки — три разные вещи

Смешение DoD и критериев приёмки — типичная ошибка: в DoD попадают строчки вида «возврат работает», и документ перестаёт быть переиспользуемым.

Пример работающего DoD

## Definition of Done (уровень задачи)
1. Код смержен в main; ветка удалена.
2. Отревьюен минимум одним владельцем затронутой области (CODEOWNERS).
3. Автотесты: покрыт новый код и хотя бы один негативный сценарий; suite зелёный.
4. Статанализ и сканер зависимостей — без новых blocking-находок.
5. Флаги: новая функциональность за флагом, значение по умолчанию — выключено.
6. Наблюдаемость: добавлены метрики/логи, позволяющие ответить «работает ли это в проде».
7. Миграции обратимы и применяются до выката кода.
8. Обновлены README/ADR/OpenAPI, если менялись контракты или решения.
9. Задеплоено на stage и проверено вручную по «как проверить» из PR.
10. Тикет закрыт со ссылкой на PR; известные ограничения записаны отдельными тикетами.

## Definition of Done (уровень релиза)
11. Прогон нагрузочного теста при изменении горячего пути.
12. Дежурный уведомлён, обновлён runbook по новым алертам.
13. Флаг включён на 5% трафика, метрики ошибок стабильны 24 часа.

Свойства хорошего DoD:

• Проверяемость. Каждый пункт даёт ответ «да/нет» без спора. «Код качественный» — не пункт DoD.
• Достижимость каждый раз. Если пункт систематически нарушают — либо чините процесс, либо убирайте пункт. Мёртвый пункт обесценивает весь документ.
• Эволюция. DoD ужесточают по мере зрелости: сначала «есть тесты», через полгода «покрытие критичных путей», ещё позже «прогон в canary». Ретроспектива — естественное место для правки.
• Видимость. Живёт в репозитории и в шаблоне PR, а не в презентации трёхлетней давности.

Как DoD связан с потоком работы

Обратите внимание на две петли назад к разработке — от CI и от ревью. Чем длиннее эти петли по календарному времени, тем дороже каждая итерация. Именно поэтому ускорение ревью даёт эффект, несопоставимый с его стоимостью: оно сокращает самую дорогую петлю обратной связи в цикле разработки.

9. Варианты процесса: ревью не обязано быть блокирующим

Обязательное ревью каждого изменения перед мержем — распространённый, но не единственный и не всегда лучший вариант. Rouan Wilsenach описал полезную рамку ship / show / ask:

Смысл в том, чтобы режим выбирался по риску, а не применялся ко всему одинаково. Ветка политики «всё через два аппрува» кажется безопасной, но имеет измеримую цену: она удлиняет lead time, увеличивает размер батчей и обучает команду штамповать. Как замечает Мартин Фаулер в PullRequest, сам по себе pull request — инструмент, изначально придуманный для open-source, где контрибьюторам не доверяют по умолчанию; внутри команды с высоким доверием ограничения могут и должны быть слабее.

Альтернативы и дополнения:

• Парное/групповое программирование — ревью в реальном времени. Формально удовлетворяет требованию «второй пары глаз» (это явно допускают многие аудиторские практики), даёт мгновенную обратную связь и снимает задержку очереди. Цена — синхронное время двух людей.
• Post-commit review — стандарт для многих open-source проектов и для trunk-based команд: изменения идут в main, ревьюются после, серьёзные замечания оформляются отдельным PR.
• Мерж-очередь (merge queue) — гоняет CI на итоговом состоянии main, а не на устаревшей ветке. Лечит класс «у меня было зелено, а в main красно».
• Автоматизированное ревью-бот-правило — Danger и подобные проверяют мета-свойства PR: есть ли описание, не превышен ли размер, обновлён ли CHANGELOG, затронуты ли миграции без ADR.

Про то, как эти режимы соотносятся с ветвлением, лучше всего написано на trunkbaseddevelopment.com; связь короткоживущих веток с производительностью команд подтверждена исследованием DORA (см. книгу Accelerate и dora.dev).

10. Типичные ошибки и антипаттерны

1. Ревью-бутылочное горлышко. PR висят днями, автор переключается, копится WIP. Лечение: дежурный ревьюер на день, лимит открытых PR на человека, SLA на первый ответ (не на аппрув!), уведомления в канал команды, а не в личку.

2. LGTM-штамп. Аппрув за 40 секунд на 1200 строк. Обычно это не лень, а рациональная реакция на невозможную задачу — см. кривые выше. Лечение начинается с размера PR, а не с призывов к ответственности.

3. Bikeshedding (закон тривиальности Паркинсона). Двадцать комментариев про имена переменных и ноль про схему транзакций: закон тривиальности гласит, что люди тратят время пропорционально своему пониманию, а не важности вопроса. Лечение: автоформат, метка nit, правило «сначала прочитай целиком, потом комментируй».

4. Ревью как gatekeeping. Ревьюер требует, чтобы код был написан так, как написал бы он сам. Лечение — стандарт Google: одобрять то, что улучшает систему, а не то, что идеально. Если решение просто другое, но не хуже — это suggestion (non-blocking).

5. Смешанный диф. Рефакторинг + фича + переименование + обновление зависимостей в одном PR. Не ревьюится в принципе. Лечение: разделять, не стесняясь трёх PR подряд.

6. Отсутствие контекста. PR с заголовком «фиксы» и пустым описанием. Ревьюер тратит 20 минут на реконструкцию задачи. Лечение: шаблон PR, ссылка на тикет, бот, блокирующий пустое описание.

7. Ревью только «сверху вниз». Только сеньоры ревьюят джуниоров. Теряется половина смысла: джуниор, читающий чужой код, учится быстрее, а сеньор получает вопросы «а почему так?», которые находят реальные проблемы.

8. Метрики как цель. Считать «комментарии на PR» или «число ревью на человека» — прямой путь к закону Гудхарта: начнут писать комментарии ради счётчика. Полезные метрики — потоковые, и смотреть на них надо как на симптомы, а не как на KPI:

Для оценки продуктивности команд в целом полезнее многомерная рамка SPACE (Forsgren и др., ACM Queue, 2021), которая явно предостерегает от одномерных показателей.

9. Игнорирование эмоциональной стоимости. Публичная критика без объяснений, сарказм, «ты вообще читал наш стайлгайд?». Обходится дороже любого бага: люди начинают дробить PR не для читаемости, а чтобы избежать конкретного ревьюера. Хороший разбор человеческой стороны — серия How to Do Code Reviews Like a Human Майкла Линча.

10. DoD, который никто не выполняет. Документ из 25 пунктов, из которых команда фактически делает 6. Это хуже отсутствия DoD: он приучает игнорировать формальные соглашения в принципе. Сократите до выполняемого минимума и ужесточайте постепенно.

11. Как это выглядит в проде

Несколько устоявшихся конфигураций — не как образец для копирования, а как иллюстрация того, что процесс подгоняется под масштаб и риск.

Google. Одно изменение (CL) — одно логическое действие, медиана очень мала. Для мержа нужны три вещи: LGTM от компетентного ревьюера, аппрув владельца из OWNERS затронутого каталога и «readability»-аппрув от человека, сертифицированного по стилю данного языка. Скорость ответа считается культурной нормой, а не пожеланием. Инфраструктура (Critique, presubmit-проверки) делает механику почти бесплатной. Подробности — в бесплатной онлайн-книге Software Engineering at Google, глава про code review.

Open source (Linux, Kubernetes, Rust). Ревью публичное и часто многоступенчатое: sanity-проверка ботом, ревью по областям (OWNERS/CODEOWNERS), аппрув мейнтейнера, автоматический мерж-робот. Здесь ревью выполняет ещё и функцию доверия: контрибьютор внешний, его код нельзя принимать по умолчанию.

Небольшая продуктовая команда (5–8 человек). Часто оптимальна конфигурация: trunk-based, короткие ветки, один аппрув, SLA на ответ 4 часа, ship/show/ask по риску, весь стиль отдан форматтеру, обязательный merge queue. Ревью занимает 30–60 минут в день на человека — это нормальная и планируемая часть нагрузки, а не «когда будет время».

Регулируемая среда (финтех, медицина). Ревью — не только инженерная, но и аудиторская практика: требуется доказуемый след «изменение проверено человеком, отличным от автора». Тогда обязательность аппрува не обсуждается, но всё остальное (размер PR, автоматизация, скорость) остаётся полностью в вашей власти — и именно там появляется выигрыш.

Общий знаменатель всех работающих конфигураций один: дешёвое и быстрое ревью маленьких изменений, вся механика отдана машинам, спорные вопросы решены заранее записанным стандартом.

12. Мини-итог

• Ревью — канал коммуникации, а не контроль качества. Основная его продукция — распространение знаний, читаемость и общий стандарт; поиск багов важен, но вторичен и сильно перекрывается автоматикой.
• Размер PR — главный управляемый параметр. 50–400 строк: выше — качество проверки падает, а вероятность штампа растёт. Уменьшать размер учат Parallel Change, флаги, stacked PRs и разделение рефакторинга и фичи.
• Скорость первого ответа важнее полноты разбора. Цель — часы, а не дни; допустим частичный набор комментариев.
• Всё механическое — до человека. Формат, стиль, типы, секреты, CVE, тесты — pre-commit и CI. Каждый комментарий про пробелы — дефект процесса.
• Читайте PR по слоям: нужно ли это → дизайн → корректность → тесты → эксплуатация → читаемость → мелочи. Комментарии слоя 7 при проблеме на слое 2 — потраченное впустую внимание.
• Помечайте блокирующее и необязательное явно (Conventional Comments), объясняйте «почему», критикуйте код, одобряйте улучшение, а не совершенство.
• Стандарт — записанное решение, снимающее повторный спор. Он ценен единством, а не правильностью; и он должен проверяться автоматически, иначе это пожелание.
• Definition of Done делает слово «готово» однозначным. Пункты проверяемы, выполнимы каждый раз, живут в репозитории и ужесточаются по мере зрелости.
• Процесс подбирается по риску, а не по привычке: ship/show/ask, пары, post-commit review, merge queue — законные варианты.
• Метрики — потоковые и как симптомы: время до первого ответа, размер PR, lead time, DORA. Любая метрика, ставшая целью, перестаёт быть метрикой.

Источники

• Alberto Bacchelli, Christian Bird. Expectations, Outcomes, and Challenges of Modern Code Review, ICSE 2013
• Caitlin Sadowski и др. Modern Code Review: A Case Study at Google, ICSE-SEIP 2018
• Peter Rigby, Christian Bird. Convergent Contemporary Software Peer Review Practices, FSE 2013
• Google. Engineering Practices: Code Review — стандарт ревью, скорость, маленькие CL
• Titus Winters, Tom Manshreck, Hyrum Wright. Software Engineering at Google — доступна онлайн бесплатно
• SmartBear. Best Practices for Peer Code Review — сводка данных исследования в Cisco
• Conventional Comments — конвенция разметки замечаний
• Rouan Wilsenach. Ship / Show / Ask, martinfowler.com
• Martin Fowler. PullRequest, FrequencyReducesDifficulty
• Michael Lynch. How to Do Code Reviews Like a Human
• Chelsea Troy. Reviewing Pull Requests
• The Scrum Guide 2020 — Definition of Done как обязательство
• Michael Nygard, шаблоны Architecture Decision Records
• Nicole Forsgren и др. The SPACE of Developer Productivity, ACM Queue 2021
• DORA: четыре ключевые метрики и книга Accelerate
• Trunk Based Development, pre-commit, Danger, GitHub CODEOWNERS

Что дальше

На этом трек «Принципы разработки» закончен. Пройденный путь стоит увидеть целиком: от вопроса, зачем принципы вообще нужны через SOLID, DRY/KISS/YAGNI и связанность со связностью к чистому коду, рефакторингу, тестированию, twelve-factor и отказоустойчивости — и, наконец, к процессу, который удерживает всё это в живой команде.

Принципы — это язык, но не словарь готовых решений. Дальше есть три естественных направления:

• Готовые решения повторяющихся задач проектирования — если принципы говорят «уменьшай связанность», то паттерны показывают конкретные способы это сделать. Трек «Паттерны проектирования» и следом «Архитектурные паттерны» — прямое продолжение этой статьи и всего трека.
• Фундамент под кодом — структуры данных и алгоритмы: никакая чистота кода не спасёт от неверно выбранной структуры данных, а «сложность» из разбора trade-offs выше — именно оттуда.
• Язык, на котором всё это писать — треки Go, TypeScript, C# и Elixir: в каждом из них принципы этого трека выглядят немного по-своему, и в каждом есть свой раздел про SDLC и практики.

Общая карта всех треков портала и рекомендованный порядок прохождения — в дорожной карте.