Суть Читай реальные диффы, описание PR и комментарии ревью — поймай дизайн-баг, который обязан увидеть ревьюер, оцени читаемость PR и перепиши комментарий так, чтобы он называл важность и фикс.
Высота — путь к senior
НольJuniorMiddleSenior
Ты на senior-высоте — в орбите
◷ 14 min
Ревью — это чтение, а не пересказ. Вот артефакты, с которыми ты реально сталкиваешься в PR — дифф, описание, тред комментариев. Для каждого делай то, что делает senior-ревьюер: найди дефект, который тулинг не поймает, оцени, читаем ли вообще дифф, и преврати туманный комментарий в действенный.
Цель
Отработай цикл ревьюера на реальном материале: отдели неразрешимый дизайн-изъян от механического шума, читай форму PR на читаемость и пиши фидбэк, отвечающий на «насколько важно» и «что делать».
Сниппет 1 — путь ретрая, который ревьюер обязан поймать
# PR: "Add billing retry on transient gateway errors"def charge_customer(order_id, amount): record = db.insert_charge(order_id=order_id, amount=amount, status="pending") for attempt in range(3): resp = gateway.charge(amount) # may time out AFTER charging if resp.ok: db.update(record, status="settled") return resp time.sleep(2 ** attempt) # back off and retry db.update(record, status="failed") raise ChargeError(order_id)
Викторина
Completed
CI зелёный — формат, lint, типы и юнит-тесты проходят. Какой дефект ревью существует, чтобы поймать именно здесь?
Heads-up Тайминг backoff — деталь тюнинга, а не дефект. Баг корректности в том, что ретрай после таймаута-уже-после-списания списывает с клиента дважды — ровно тот неразрешимый изъян денежного пути, ради которого нужно человеческое ревью.
Heads-up Тесты проверяют пути, которые автор вообразил; они зелёные именно потому, что никто не протестировал кейс таймаута-после-успеха. Поэтому ревью целится в замысел и денежный путь, а не только в то, проходит ли набор тестов.
Heads-up Это style/typing-nit, которым должен владеть linter или система типов — разрешимо и дёшево. Потратить ревью на него, пока двойное списание уезжает в прод, — тот сбой инвертированных приоритетов, о котором предупреждает юнит.
Сниппет 2 — описание PR
Title: Refactor checkout + add coupons + migrate orders tableFiles changed: 47 +1,812 −940Description: "Cleaned up checkout, added coupon support, and migrated theorders table to the new schema. All in one PR so it deploys atomically."
Викторина
Completed
Как назначенный ревьюер, какой senior-ответ на этот PR ещё до прочтения хотя бы строчки диффа?
Heads-up Внимание — фиксированный бюджет, не масштабируемый размером диффа: за пределами нескольких сотен строк ты скользишь независимо от усилий, а реальный риск миграции прячется в непрочитанной массе. Фикс — декомпозиция, а не больше силы воли.
Heads-up Атомарный деплой — реальный концерн, но он решается порядком/feature-флагами stacked PR, а не одним нечитаемым диффом. «Деплоится атомарно» — рационализация, которая порождает rubber stamp на самом рискованном изменении.
Heads-up Каждый добавленный ревьюер всё равно видит 1800 строк за пределами своего когнитивного лимита — больше скольжений и больше ожидания подхвата, а не больше обнаружения. Лимит внимания на человека — ограничение; размер диффа — рычаг.
Сниппет 3 — комментарий ревью
Reviewer comment on a 40-line PR, left as one line on the file: "this is confusing, why did you even do it this way"
Викторина
Completed
Почему этот комментарий породит трение и какова минимальная переписка, которая его чинит?
Heads-up Длина — не проблема, проблема — ясность. Короткий комментарий, называющий важность и конкретное действие, бьёт длинный, который всё равно оставляет автора гадать. Фикс — лейбл плюс действенный запрос, а не больше слов.
Heads-up Прямота, атакующая автора и опускающая важность, создаёт round-trips и подтачивает психологическую безопасность, которая держит людей в готовности искать ранний фидбэк. В ожидании это медленнее, а не быстрее.
Heads-up Если есть настоящий концерн по читаемости, поднять его правильно — ревью и про maintainability тоже. Дефект в том, КАК сформулировано, а не в том, что подняли; пометь его и сделай действенным.
Сниппет 4 — дифф, который никогда не должен доходить до человека
- const userName=req.body.userName+ const userName = req.body.userName;- function getUser( id ){return db.find(id)}+ function getUser(id) { return db.find(id); }
Викторина
Completed
Это единственные изменения в PR, и ревьюер потратил десять минут на ручную проверку пробелов и точек с запятой. Что это говорит о пайплайне?
Heads-up Ручная проверка стиля непоследовательна и это повторяющийся налог на каждый PR; formatter применяет те же правила одинаково и мгновенно. Гонять разрешимые проверки через человека — медленная, ошибкоёмкая версия инструмента.
Heads-up Изменения форматирования нормально мержить — суть в том, что их должен производить и проверять formatter в CI, а не человек вручную. Запрещай ручное ревью их, а не сами изменения.
Heads-up Это показывает, что у команды нет гейта форматирования, а не что автор неполноценен. С автоформатом при сохранении и блоком в CI ни один человек никогда не видит и не судит этот класс изменений.
Итог
Чтение — это и есть работа: зелёный CI и проходящие тесты не сертифицируют денежный путь, поэтому ревьюер охотится за неразрешимым дефектом — здесь это неидемпотентный ретрай, списывающий дважды. Форму PR читают до диффа: изменение в 1812 строк на три концерна делят или стекают, потому что внимание до него не дотянется. Комментарии судят по тому, называют ли они важность и фикс и целятся ли в код, а не в автора. А дифф, полный правок пробелов, проверяемый человеком вручную, — это отсутствующий гейт форматирования: разрешимый класс никогда не должен доходить до ревью. Диагностируй по артефакту, чини на правильном слое, держи человеческое внимание на том, что может решить только человек.