Привет, Хабр!
Сегодня хотелось бы поговорить о код-ревью, но не о коде. Думаю, не я один такой, и многие замечали, что если пишешь код для себя, то спокойно (ну не всегда, конечно) переписываешь его хоть десять раз. Но как только в твой код приходит кто-то и пишет что-то вроде "сделай здесь по-другому", то у тебя запросто может полыхнуть. Ты же старался, писал этот шедевр дня три. Там какая-нибудь сложная логика, много краев, да и вообще гордишься ("я сделяль", да). А тебе прилетает список из двадцати замечаний, и хорошо если без едкого комментария. И вот ты готов биться как лев за каждую строчку. Стараешься придумать достойный ответ (возможно, даже саркастичный). Потом серия препирательств, кто правее, а в итоге вообще может оказаться, что кто-то из вас изначально чего-то недопонял в задаче.
Собственно, к чему это я? Просто сейчас мне кажется, что код-ревью - это вообще-то не только про код. Это еще и про людей.
Почему же иногда так бесит, когда трогают твой код? Есть такая штука - "IKEA-эффект". Мы склонны переоценивать то, что сделали своими руками. Думаю, для кода это работает даже сильнее. Потому что код - это не мебель. Это же твои мысли, твои решения, твои бессонные ночи. И когда получаешь комментарий "переделай", мозг превращает его в "ты плохо подумал".
Еще есть забавная штука - ошибка атрибуции. Свои ошибки мы всегда объясняем внешними обстоятельствами (дедлайн полыхал, требования поменялись, сложный легаси и т.д). А вот чужие ошибки кажутся результатом невнимательности или недостатка знаний. Про это надо помнить и когда ты ревьюер, и когда ты автор.
Мне кажется, я научился замечать момент, когда падает забрало и я перехожу в защиту, так сказать. Это даже физически ощущается. Тут и сердце начинает стучать быстрее, и хочется ответить как-то едко. Это значит, что самое время пойти попить чаю и выдохнуть. Минуток десять хотя бы подождать. Почти всегда оказывается, что комментарий был не такой уж обидный и даже полезный.
И да, принимать критику спокойно не так уж просто. Но надо постараться держать в голове мысль: "Критикуют не меня, а строчки". Иногда, чтобы отстраниться, читаю комментарий и представляю, что код вообще не мой, а моего, допустим, друга. Ну или уважаемого коллеги. И тогда реагирую уже по-другому. Вместо того, чтобы уйти в глухую защиту, начинаю думать: а что бы я посоветовал автору?
В общем, тут лучше бы стараться без эмоций, чтобы не впадать в крайности. Если согласен, лучше просто написать "спасибо, поправил" – этого достаточно. Без расшаркиваний и самобичевания (ну и, уж разумеется, без попыток отстоять очевидный баг). Если не согласен, то объяснить почему (ну перед этим, если требуется, вдох-выдох, чаю попить и вот это все). И стараться не спорить о вкусах. "А мне так нравится" вообще не аргумент, так что надо подробно объяснить – почему нравится-то? В чем преимущества? Если же, допустим, неуверен - задать вопрос. Спрашивать не стыдно (и даже если ты сеньор). Лучше все же спросить, чем потом переделывать.
Самое сложное - когда точно знаешь, что прав, а ревьюер таки настаивает на своем. Как-то раз мы так часа полтора спорили, в итоге позвали третьего, из другой команды. Он поглядел, сказал, что оба варианта вполне себе ничего, и быстро придумал компромисс.
Ну а если со стороны ревьюера посмотреть… Можно просто задать себе вопросы перед тем, как комментить:
А если б мне такое написали, я бы расстроился?
А автор знает то, что знаю я? (О каких-то особенностях, ограничениях и т.п.)
А это прям ошибка или моя вкусовщина? (Этот вопрос, наверное, самый важный.)
Вообще комментарии можно даже поделить на категории. Например, блокер (это когда прям баг, уязвимость, нарушение архитектуры) и все остальное (переименовать переменную, поправить отступ, улучшить читаемость). Потому что второе – это не всегда обязательное к исправлению. Это не критично.
Разумеется, не стоит использовать повелительный тон. Это типа "переименуй переменную", "вынеси это в метод" и так далее. Лучше как-то вопросительно или описательно.
Ну допустим:
"Тут трижды повторяется одна и та же логика. Может, вынести в отдельный метод?"
"Думаю, имя слишком общее. А что если назвать по-другому, чтобы было понятнее?"
Разница в том, как это воспринимается. Ваша цель - вместе найти решение, а не выяснить кто тут самый умный.
Разумеется, хвалить тоже надо. Но лучше прям конкретно. Не "все ок", а что именно сделано хорошо. Ну и добавить "спасибо", конечно, и человеку сразу приятнее станет. Потому что конкретная похвала работает намного лучше, чем общая.
Если ревью пошло уже на третий-четвертый круг, это уже, скажем так, штопор какой-то. Вы больше не обсуждаете код, вы выясняете, кто правее. Так что, если после двух раундов ревью прогресса не наблюдается, лучше созвониться или встретиться. Скорее всего, это все решит. Потому что в тексте мы не видим интонации, не можем быстро уточнить, не можем набросать на доске. А голосом все разрулится за пять минут.
Ну а если ревьюер злоупотребляет статусом, можно вежливо попросить аргументы. Обычно это работает. Если не работает, это уже проблема команды, и ее нужно решать на ретроспективе. Допустим, раз в месяц анонимно собирать обратную связь про ревью. Просто три вопроса: "Что было полезно?", "Что бесило?", "Что изменить?".
В общем, если кратко, то правила простые (казалось бы, это очевидности такие, но порой о них забывают):
Автору:
не принимай критику на личный счет
отвечай по делу, без оправданий или грубостей
благодари за дельные замечания
Ревьюеру:
отделяй блокеры от мелочей (и тем более придирок)
задавай вопросы, а не указывай
хвали за хорошие решения
Собственно, если после ревью разработчик расстроен и демотивирован, думаю, ревьюер отчасти провалил свою задачу. Даже если код стал идеальным. Но и автор должен помнить, что ревьюер тоже человек. Он тоже может устать, быть в запарке, да может, день у него плохой сегодня, в конце концов. И его тон в комментарии - это не про твою ценность как разработчика. В общем, помните, вежливость, уважение и готовность услышать другого всегда помогут найти решение.