Soft skills в code review

Вы научитесь писать замечания, которые автор принимает без спора; отличать разногласие по существу от разногласия из-за формы; выходить из тупика в переписке до того, как тред разрастётся до пятидесяти комментариев; и принимать критику своего кода, не тратя силы на защиту.

Приоритеты замечаний (blocker / major / nit) разобраны в теме Принципы code review — здесь речь о том, как формулировать содержание внутри этих уровней.

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

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

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

Проверьте себя. Перечитайте свой последний десяток комментариев и отметьте те, в которых нет причины.

Частая ошибка. Считать мягкие формулировки уступкой качеству. Требовательность живёт в уровне замечания, а не в тоне.

Четыре элемента, из которых обычно нужны три:

Наблюдение — что конкретно в коде. Следствие — что из этого произойдёт и когда. Предложение — как можно иначе. Уровень — блокирует или нет.

Отдельная сила у формы вопроса. Она уместна, когда вы не уверены, что видите всю картину, — а это чаще, чем кажется.

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

Проверьте себя. Возьмите замечание, которое собирались написать как утверждение, и переформулируйте вопросом. Изменилось ли ваше собственное понимание?

Частая ошибка. Задавать вопрос, который на самом деле утверждение: «а ты тесты запускал?». Это упрёк в маскировке, и он читается именно так.

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

Важно, чтобы похвала была конкретной. «Хороший PR» не несёт информации; «удачно, что вынес расчёт в чистую функцию — теперь на него можно писать параметризованные тесты» — несёт.

Проверьте себя. В последних пяти проведённых вами ревью — сколько было отмеченных удачных решений?

Частая ошибка. Хвалить в начале, чтобы смягчить критику. Формула «похвала — критика — похвала» распознаётся, и похвала обесценивается.

Замечание senior-разработчика junior'у — не разговор равных, даже если по форме выглядит так. Автор часто соглашается не потому, что убедился, а потому, что не решается спорить с более опытным. Итог: правка внесена, понимание не появилось, ошибка повторится.

Что помогает:

В обратную сторону тоже: junior, ревьюящий senior'а, часто ограничивается LGTM, потому что не чувствует права возражать. Между тем «я не понял, что здесь происходит» — полноценное и очень ценное замечание: если код непонятен новому человеку, это факт о коде, а не о человеке.

Проверьте себя. Когда вы последний раз писали в PR «я не понимаю этот фрагмент»? Если никогда — вероятно, вы иногда одобряете то, что не разобрали.

Частая ошибка. Считать непонимание своей проблемой и молчать. Ревью существует в том числе чтобы это обнаруживать.

Реакция «на мой код напали» физиологична, спорить с ней бесполезно. Помогает не самоконтроль, а несколько привычек.

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

Отвечать на все комментарии. Даже отклонённые: «оставляю как есть, потому что …». Молча проигнорированное замечание — главный источник обиды у ревьюеров и причина, по которой они начинают перепроверять каждую строчку.

Уточнять, а не защищаться. «Правильно ли я понял, что предлагается вынести это в сервис?» стоит дешевле, чем два раунда правок в неверном направлении.

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

Различать «я не согласен» и «мне неприятно». Первое надо обсуждать, второе — переждать, а потом посмотреть на замечание ещё раз.

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

Частая ошибка. Внести правку, не поняв её смысла, чтобы быстрее закрыть PR. Через месяц то же решение появится снова.

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

Три способа выхода:

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

Разделить решения. Часто спор идёт о двух вещах одновременно: «здесь баг» и «архитектура неудачная». Первое решается в этом PR, второе — тикетом. Смешивание их делает разговор безвыходным.

Эскалация без драмы. «Мы не сходимся, давай позовём третьего» — нормальная процедура, а не жалоба. Важно позвать до того, как тред станет неприятным.

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

Проверьте себя. Найдите в истории проекта тред длиннее двадцати комментариев. Чем он закончился и сколько времени занял?

Частая ошибка. Продолжать переписку, потому что «осталось совсем немного, сейчас я его убежу».

Реальная по структуре переписка в PR, добавляющем экспорт в CSV. Автор — разработчик первого года.

Что произошло. Формально ревьюер прав: join уместнее. Фактически проделана худшая из возможных работ.

Первая реплика содержит «так никто не делает» — оценку автора, а не кода, и ноль информации. Ответ «а что не так?» — прямое следствие: автору не сказали, что не так.

Вторая реплика отправляет читать вместо того, чтобы назвать причину в одну строку. Это читается как «разберись сам, мне некогда объяснять».

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

Финальное «хорошо» — не согласие, а выход из неприятного разговора. Правка будет внесена, ничего не понято, и в следующий раз автор просто не станет спорить.

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

Как это выглядит иначе:

Три отличия. У каждого замечания указан уровень, поэтому автор видит, что важно. У каждого названа причина, поэтому обсуждение идёт по существу. Мелочь честно помечена мелочью — и как раз поэтому не потянула за собой спор.

Как устроить процесс, в котором такие разговоры происходят по умолчанию, — курс Code Review для команды: культура и разрешение конфликтов.