Комментарии на ревью: как сказать «нет» и не обидеть

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

Ошибка здесь не роняет прод. Она приводит к тому, что коллега перестаёт звать вас в обсуждения, ревьюит ваши PR последними, а через полгода на калибровке кто-то произносит hard to work with — и вы никогда не узнаете, что эта фраза была сказана.

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

Почему ревью ломается чаще всего

Асимметрия усилий. Автор потратил на код три дня, ревьюер на комментарий — двадцать секунд, и автор читает эти двадцать секунд как оценку трёх дней жизни. Why not just use a map? для одного мимолётная мысль, для другого — «ты три дня делал ерунду, решение было очевидным».

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

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

Код, на котором будем тренироваться

# handlers/orders.py — фрагмент, пришедший на ревью
def get_user_orders(user_id):
    # выбираем всю таблицу заказов
    rows = db.execute("SELECT * FROM orders")
    result = []
    for r in rows:
        # фильтруем уже в памяти приложения
        if r.user_id == user_id:
            result.append(serialize(r))
    return result

Дефект бесспорен: полная выборка вместо WHERE в запросе, 200 тысяч строк в проде, вызов на каждый запрос списка заказов. Именно поэтому фрагмент — хороший полигон: техническая правота не спасает формулировку. Вот пять комментариев об одном и том же, от разрушительного к рабочему.

1. This is horrible. Never SELECT *.
2. Why did you fetch the whole table?
3. just add a WHERE clause
4. Could we filter in SQL?
5. issue (non-blocking): this loads the whole orders table into memory on
   every request — with 200k rows we would OOM in prod. Could we push the
   filter into the query, or page it? Happy to pair on this if it gets fiddly.

(1) horrible — оценка кода, которая читается как оценка автора; Never — императивный запрет от человека, который вам не начальник. Ни причины, ни выхода: информативно ровно на одно слово.

(2) Why did you...? в рабочем регистре — почти всегда обвинение, требующее оправдания. Русское «а почему ты взял всю таблицу?» нейтрально, английский аналог значит «объяснись». Если причина нужна по-настоящему, спрашивают без обвиняемого в подлежащем: What led you to load it all up front?

(3) just — самый вредный односложный элемент инженерного английского: утверждает, что решение тривиально, а значит, автор не справился с тривиальным. Строчная буква и отсутствие точки добавляют «мне лень оформлять реплику для тебя».

(4) Уже приемлемо: вопрос, Could we, местоимение we вместо you. Но нет силы (блокер или пожелание?) и нет причины, а без причины замечание попадает в разряд вкусовщины. (5) Рабочий вариант; он длиннее — и это нормально: двадцать лишних секунд ревьюера экономят полдня недопонимания.

Анатомия рабочего комментария

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

1. Маркер силы — issue (non-blocking):. Отвечает на «обязательно ли это».
2. Наблюдение — this loads the whole orders table into memory: факт о коде без прилагательных-оценок. Факт не спорится, оценка спорится.
3. Последствие — with 200k rows we would OOM in prod. Превращает вкусовщину в инженерный аргумент; самый часто пропускаемый слот.
4. Выход — Could we push the filter into the query, or page it? Конкретные варианты, поданные вопросом; решение остаётся за автором.
5. Место автору — Happy to pair on this if it gets fiddly. В мелкой правке опускается, в крупной — нет.

Проверка перед отправкой: уберите всё, кроме глаголов и объектов, — о чём осталось утверждение? Если о человеке, а не о коде, переписывайте.

Маркеры силы: главный инструмент главы

Основной источник недоразумений — не грубость, а неопределённость: автор не понимает, что от него требуется. Отрасль решила это явными префиксами; кодификация — Conventional Comments, формат label (decorations): subject.

nit — от nitpick, «придирка»; ставя его, вы безвозвратно отдаёте решение автору, и написать nit:, а потом не апрувить из-за этого замечания — нарушение контракта, которое замечают. question: опаснее, чем кажется: настоящий вопрос и риторический («зачем здесь ретрай?!») выглядят одинаково, поэтому называйте, чего вам не хватает: question: I don't have context on the retry policy — is 5 attempts a product requirement or a guess?

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

Отдельно — кнопка. Статус ревью (Comment, Approve, Request changes) — сигнал сильнее любых слов. Классический конфликт: мягкий текст плюс Request changes (читается как «заблокировали за придирку») или жёсткий текст плюс Approve (непонятно, надо ли что-то делать). Держите кнопку и текст в одном регистре; процессуальная сторона — в главе совместная работа, язык описания самого PR — в главе https://courses.digitable.life/post/engineering-english/04-commits-and-prs/.

Как читать чужие комментарии

Обратная задача сложнее: нужно извлечь силу требования из формулировки, которая её намеренно прячет.

На ревью любой вопрос о вашем коде — это запрос на действие, пока явно не сказано обратное.

Мягкая форма — стандартная упаковка жёсткого требования: смягчение адресовано вашему лицу, а не сути.

Маркеры настоящей необязательности всегда явные — язык не оставляет опциональность на догадку: nit:, optional:, not blocking, feel free to ignore, up to you, no strong opinion, take it or leave it, FWIW, out of scope, ignore for now. Нет ни одного — считайте комментарий требованием: ошибка в эту сторону стоит десяти секунд на ответ, ошибка в другую — двух дней.

Британская недоговорённость

Одна и та же фраза от американца и от британца означает разное — в смешанной команде это ежедневная проблема.

Универсальный выход из такой неопределённости — прямая фраза, которая в инженерной культуре никого не оскорбляет: Just to be explicit: is this a blocker for you, or something I can follow up on separately? Она просит не изменить тон, а уточнить статус, и инженеры отвечают на неё охотно.

Дозировка смягчения

Смягчение — не украшение, а регулятор громкости. Проблема русскоязычных инженеров симметрична: в тексте мы недосмягчаем, в устной речи пересмягчаем (про устную сторону — https://courses.digitable.life/post/engineering-english/08-meetings/).

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

Плохо:  Maybe we could possibly consider perhaps moving this out of the handler?
        (maybe + could + possibly + perhaps = четыре)
Хорошо: Could we move this out of the handler?

Ответ на первую строку — вежливое Good point!, после которого не происходит ничего: формулировка сама сообщила, что автор в неё не верит.

Правило второе: смягчают требование, а не факт.

Плохо:  I think this might possibly return None when the cache is cold.
Хорошо: This returns None when the cache is cold.
        Should we fall back to the DB, or is None expected here?

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

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

Как сказать «нет»: три сценария

А. Вы ревьюер и отклоняете подход целиком

Дефект не в строке, а в решении: исправление означает переделку, и формы мало — нужна структура.

Thanks for picking this up — the caching part reads really well.

I do think the data access should go through the repository rather than raw SQL
in the handler. Two reasons: everything else in this service moved that way last
quarter, and the next person touching this will look for the query there.

That's a fair amount of rework, so before you start — does that match your
understanding, or is there a constraint I'm missing? `OrderRepository.find_recent_by_user`
is a close analogue if you want a starting point.

• Конкретная похвала первым абзацем, а не Great job!: общая похвала перед критикой мгновенно узнаётся как «бутерброд» и обесценивает обе части.
• I do think — эмфатическое do даёт ясную позицию без агрессии: субъект назван, мнение помечено как мнение, но сила есть.
• Две внешние причины — согласованность и стоимость сопровождения: аргумент, который нельзя свести к вкусу, невозможно отбить вкусом. Плюс признание цены (That's a fair amount of rework): одно предложение, показывающее, что вы понимаете, о чём просите.
• Выход собеседнику — is there a constraint I'm missing? Не риторический: лучше узнать о неизвестном вам ограничении до переделки, а не после.
• Конкретная помощь — ссылка на аналог; отклонить подход и не дать направление значит оставить человека наедине с чистым листом.

Б. Вы автор и не согласны с ревьюером

Работает лестница: каждая ступень дороже предыдущей и берётся, только если не сработала предыдущая.

• Just to make sure I understand — you'd like X, right? — переформулировка чужой позиции своими словами: не вежливость, а диагностика, часто выясняется, что спорить было не о чем.
• Does that change your view? — прямой запрос на пересмотр; без него ветка виснет: вы высказались, а ревьюер не понял, ждут ли от него ответа.
• I don't feel strongly — if you do, I'll change it. — самая полезная фраза в арсенале инженера: честно сообщает вес вашей позиции и передаёт решение, не капитулируя. Когда решили не по-вашему, работает отраслевая формула disagree and commit: I still think X, but I'm fine going with your call — let's not block the release on this. Чего делать нельзя — молчать: непринятый комментарий без ответа читается как «проигнорировал».

В. Вы не согласны со старшим коллегой или архитектором

Форма та же, меняются две вещи: объём доказательств и выбор площадки.

You have a lot more context on the migration than I do, so I may be off here.
My concern is narrow: if we drop the compatibility shim in this PR, the mobile
clients on 4.2 stop working, and 4.2 is still ~12% of traffic. Would it be
reasonable to keep the shim until the 4.2 share drops below 2%, or is there
a reason it has to go now?

Работают четыре вещи: признание асимметрии контекста (не подхалимство, а факт — вы правда можете чего-то не знать, и это снимает с собеседника необходимость защищать статус); сужение спора (My concern is narrow — один названный риск вместо оспаривания решения целиком, узкое возражение принимается несравнимо легче широкого); число (~12% of traffic — единственный класс аргументов, который не отбивается опытом); просьба о решении, а не о победе (Would it be reasonable to..., с правом отклонить).

Правило площадки: не спорьте со старшим больше двух раундов публично — затяжной публичный спор заставляет вторую сторону защищать репутацию, а не позицию. Третий раунд — в личку или голосом, результат — обратно в ветку; механика таких разговоров разобрана в главах трудные разговоры и работа вверх. Если спор архитектурный и повторяющийся, выносите его из ревью в архитектурное решение: This keeps coming up in reviews — worth writing a short ADR so we decide it once?

Жизненный цикл ветки обсуждения

Опаснее всего состояние «завис»: молчание автор читает как «я ещё думаю», а ревьюер — как «проигнорировал», и через сутки это уже не техническая, а социальная проблема. Правило трёх раундов. Если ветка перевалила за три обмена репликами, текст перестал работать: вы либо не понимаете друг друга, либо спорите о ценностях, а не о фактах. Переход в звонок, не выглядящий ни капитуляцией, ни эскалацией: We seem to be going back and forth on this — want to grab 10 minutes tomorrow? Happy to go either way afterwards, I'd just like to unblock the release.

Резюме обязательно. После любого разговора вне ветки в неё пишется исход: Discussed offline: keeping the current approach for this PR, revisiting after the storage migration. Filed as PLAT-482. Без этой строки коллеги видят спор без исхода, а через полгода никто не вспомнит, почему код такой.

Ответы автора: короткий словарь

I'm not sure I follow обязано сопровождаться вашей гипотезой (do you mean X?): голое I don't understand заставит ревьюера повторить то же самое другими словами — минус ещё раунд. Отдельно про извинения: русскоязычные инженеры извиняются заметно чаще нормы, и хотя каждое Sorry безобидно, их плотность формирует образ человека, постоянно в чём-то виноватого, что влияет на восприятие его технических позиций. Рабочая замена — благодарность вместо извинения: Sorry for the delay → Thanks for your patience. Смысл сохраняется, позиция меняется.

Чего не писать никогда

• just, simply, obviously, clearly. You can just use a set here. Автор слышит «это было тривиально, а ты не догадался»; если бы было очевидно, комментарий был бы не нужен. Убирается без потери смысла: A set would give O(1) lookups here, and this runs per row.
• Why would you do this? и Did you even test this? Риторические вопросы читаются как насмешка и обвинение. Замена второго: Do we have a test covering the empty-list case?
• As I said before / per my previous comment. Узнаваемая пассивная агрессия — повторите мысль полностью со ссылкой на прежнюю ветку.
• Восклицательные знаки в претензии и сарказм. This is wrong! — крик; Nice, a fifth way to parse dates in this repo. — ирония, которая не переживает асинхронный текст даже между носителями, а между не-носителями читается буквально и злобно.
• Эмодзи вместо смягчения. Rewrite this 🙂 жёстче, чем Rewrite this: смайлик, приклеенный к требованию, воспринимается как насмешка.
• Оценка человека вместо кода. You always forget the null check. Переход на личность плюс always — обвинение в устойчивом паттерне, неопровержимое в рамках одного PR.
• We need to talk about this offline без предмета — читается как угроза. Добавьте тему: Let's chat about the retry policy offline — it's bigger than this PR.

Nit-цунами, bikeshedding и похвала

Тридцать комментариев на PR, из которых двадцать восемь про пробелы, имена переменных и порядок импортов. Формально каждый корректен; суммарно это сообщение «твоя работа никуда не годится». Три правила: больше пяти однотипных мелочей — один сводный комментарий (nit: a few naming inconsistencies across the file — worth a pass, but not blocking.); спор о стиле решается конфигом, а не людьми (если замечание может поставить линтер — поставьте линтер); помечайте несущественное явно — ревьюер, у которого 80% замечаний без маркера силы, обучает автора игнорировать их все.

Слово, которое стоит знать активно: bikeshedding — непропорциональное внимание к тривиальным вопросам. Термин пришёл из закона Паркинсона о тривиальности и закрепился в отрасли после письма Пола-Хеннинга Кампа в рассылке FreeBSD 1999 года. Употребляется самокритично и работает как разрядка: Sorry, this is bikeshedding — feel free to ignore.

Обратная сторона — похвала. Ревью по природе поток замечаний: никто не комментирует девяносто строк, написанных хорошо, и автор получает двенадцать сообщений, все о том, что он сделал не так. Лечится одним конкретным praise: на ревью; требование ровно одно — конкретность, иначе похвала читается как ритуальная подготовка к удару. Не Great job! и не Looks good overall, but ... (узнаваемый бутерброд), а praise: this test name explains the bug better than the ticket did. Сильнейшая форма — TIL you can pass a comparator here: вы признаёте, что научились у автора, и это работает даже между сильно различающимися грейдами.

Сокращения, без которых ревью читается с трудом: LGTM (Looks Good To Me — одобрение, но кнопку Approve оно не заменяет, и в командах с обязательным ревью это регулярный источник зависших PR), SGTM (согласие с планом, а не с кодом), PTAL (автор зовёт ревьюера обратно), WDYT? (запрос мнения, ждёт ответа), IMO/IMHO, AFAICT и IIRC (ограничение ответственности), FWIW и NAB (Not A Blocker — маркеры необязательности), Ship it (неформальный апрув).

Тон не равен смыслу: международный контекст

В типичной международной команде носителей меньшинство, значит, тон определяется родной культурой автора, наложенной на английские слова. Немецкие, голландские, израильские коллеги пишут This is wrong. — это не грубость, а техническая констатация. Японские, индийские, тайские коллеги могут написать This might be a little difficult или I'll try, имея в виду отказ. Американцы упаковывают отказ в энтузиазм: That's interesting! Have you thought about... — это no. Надёжная стратегия одна: судить по действию, а не по тону — изменился код или нет, нажата кнопка или нет, назван срок или нет, — а при сомнении спрашивать прямо.

Типичные ошибки русскоязычных инженеров

Отдельно про must: в спецификациях MUST — нормативное обязательство (см. https://courses.digitable.life/post/engineering-english/02-specs-and-rfc/), и эту силу переносят в комментарии, где между равными you must звучит как приказ от человека, не имеющего на него права. Замена — безличное this needs to. Общий разбор языковых калек — в главе https://courses.digitable.life/post/engineering-english/12-common-mistakes/.

Чеклист перед отправкой комментария

1. Понятно ли, обязательно ли это? Нет — добавьте nit: или blocking:.
2. Есть ли причина, а не только замечание? Причина превращает вкус в аргумент.
3. Подлежащее — код или человек? this does X вместо you did X.
4. Есть ли конкретный выход? Указать проблему и не дать направление — половина работы.
5. Нет ли just, obviously, сарказма, восклицательных знаков в претензии?
6. Сколько смягчений в предложении? Больше двух — уберите лишние.
7. Если бы это написали вам — поняли бы вы, что делать до конца дня?

Зеркальный чеклист для чтения: есть ли маркер необязательности; если нет — что от меня требуется.

Мини-итог

• Ревью опасно не языком, а асимметрией: секунды ревьюера против дней автора и ноль интонации.
• Рабочий комментарий состоит из пяти слотов: маркер силы, наблюдение, последствие, выход, место автору. Пропущенный слот читатель достраивает сам и обычно не в вашу пользу.
• Явные префиксы (nit:, question:, issue:, blocking:) снимают неопределённость требования — главную причину недоразумений; nit: при этом обязательство не блокировать. При чтении: любой вопрос о вашем коде — запрос на действие, пока явно не написано not blocking или up to you, а молчание читается как отказ.
• Смягчают требование, а не факт: одно смягчение на утверждение, два — потолок.
• Несогласие поднимается по лестнице (уточнение → аргумент → данные → явная цена уступки → разговор вне ветки), и со старшим коллегой меняется не форма, а объём доказательств и площадка.

Источники

• How to write code review comments и How to handle reviewer comments — Google Engineering Practices с обеих сторон: как писать замечания и как на них отвечать.
• Conventional Comments — формат label (decorations): subject; полстраницы, которые команда может принять за один стендап.
• Respectful Code Reviews — документ проекта Chromium с разбором конкретных фраз и их восприятия.
• How to Do Code Reviews Like a Human — Michael Lynch, две части; лучший практический разбор формулировок в открытом доступе. Оттуда же стоит перейти к письму Poul-Henning Kamp 1999 года, откуда в инженерный английский пришло bikeshedding.
• Erin Meyer, The Culture Map (2014) — ось «прямая и непрямая негативная обратная связь»: не про язык, но объясняет половину международных недоразумений на ревью.

Что дальше

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