Культура code review

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

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

Наблюдаемые признаки, которые можно проверить в своём репозитории:

Если во всём репозитории нет ни одного «я не понимаю» — это не значит, что всё всем понятно. Это значит, что признаваться в непонимании дороже, чем поставить LGTM.

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

Проверьте себя. Поищите в репозитории комментарии со словами «не понял», «не уверен», «возможно, я ошибаюсь». Сколько нашлось за год?

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

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

Отсутствие уровней у замечаний. Если nit и blocker выглядят одинаково, автор либо исправляет всё подряд, тратя день на мелочи, либо начинает игнорировать замечания целиком. И то и другое читается ревьюером как неуважение, хотя причина техническая.

Большие PR. PR на тысячу строк невозможно отревьюить внимательно. Ревьюер либо ставит формальный LGTM — и чувствует себя соучастником, — либо тратит три часа и приходит раздражённым. Оба исхода портят отношения, и ни один не решается призывом «будьте внимательнее».

Один ревьюер на всю команду. Человек, через которого проходит каждый PR, становится узким местом и постепенно — источником раздражения для всех. При этом он же выгорает первым.

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

Отсутствие срока на ответ. PR, висящий четыре дня, сообщает автору, что его работа не важна. Обычно причина не в этом, но прочитывается именно так.

Общее у всех пяти: лечится изменением процесса, а не разговорами о ценностях. Это главная мысль урока.

Проверьте себя. Какой из пяти пунктов присутствует в вашей команде? Что конкретно вы можете изменить на этой неделе?

Частая ошибка. Отвечать на культурную проблему тренингом по коммуникации, когда причина — размер PR и отсутствие SLA.

Из всех командных практик вокруг ревью калибровка даёт наибольший эффект и почти никогда не проводится.

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

Как проводится, за час:

Типичные результаты первой калибровки в реальной команде: «отсутствие теста на багфикс — всегда blocker»; «стиль и форматирование не обсуждаются вообще, это работа линтера»; «архитектурное возражение к уже написанному коду — major с тикетом, а не требование переписать сейчас»; «nit не блокирует мердж никогда, даже если ревьюер уверен».

Повторять раз в квартал и обязательно после прихода нового человека — иначе его представления останутся его собственными.

Проверьте себя. Возьмите PR с длинным тредом, расставьте уровни сами и попросите коллегу сделать то же независимо. Сравните.

Частая ошибка. Проводить калибровку на простых PR. Ценность в спорных случаях — на бесспорных все согласны и так.

В культуре ревью тимлид влияет сильнее всех — и чаще всего не тем, что говорит, а тем, что делает сам.

Что работает:

Публично признавать свои ошибки в PR. Один комментарий «ты прав, я не подумал про этот случай» от тимлида делает для безопасности больше, чем любое объявление о ценностях.

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

Пресекать личные выпады немедленно и в том же треде. Не в личных сообщениях. Реакция, которую не видно, не меняет норму — все остальные видели только выпад.

Не быть обязательным ревьюером. Иначе решение «одобрено тимлидом» перестаёт быть техническим.

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

Проверьте себя. Тимлид: когда вы последний раз просили ревью у самого junior-разработчика команды?

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

Реальная по структуре переписка. Автор — разработчик, вышедший из отпуска и не знавший об изменившихся договорённостях. Тред публичный, читают все.

Что произошло. Ни одной грубости, ни одного оскорбления. И при этом тред нанёс максимальный ущерб.

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

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

«+1, у нас так не делают» — присоединение без содержания. Для автора это не второе мнение, а перевес числом: против него теперь двое.

Ссылка на модуль orders как на образец, который при проверке оказался таким же, а затем «это legacy, не смотри туда» — автор обнаруживает, что правило применяется избирательно. Доверие к правилу исчезает: оно оказывается не нормой, а поводом.

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

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

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

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

Чем закончилось в реальности. Автор уволился через два месяца, и этот тред он назвал в выходном интервью. Формально к ревьюерам претензий не было — никто не грубил.

Команда после этого внесла три изменения: договорённости фиксируются в CONVENTIONS.md в репозитории и меняются только через PR; «+1» без содержания в тредах не используется; сомнения в целесообразности задачи высказываются до начала работы, а в ревью обсуждается реализация.

Одно изменение на выбор, любое из них занимает меньше часа:

Как устроить процесс, поддерживающий эту культуру, — Процесс и workflow. Что делать, когда несогласие не разрешается, — Разрешение конфликтов. Личные навыки формулировок — Soft skills в code review.