![](https://habrastorage.org/webt/a0/qy/ly/a0qylycy5x6znbawicj2tascun0.png)
Код-ревью частенько порождают споры. При подготовке лекции «Отучаемся от токсичного поведения на код-ревью» на конференции AlterConf я была готова услышать кучу возражений и критики. Но совершенно не ожидала, что сообщество настолько поддержит идею. Я предполагала сопротивление, но сообщество очень доброжелательно и с одобрением приняло меня.
Меня попросили поделиться слайдами, но теперь я подумала, что слайды сами по себе малополезны и вырваны из контекста: им не хватает объяснений. Поэтому решила опубликовать эту статью. Позже организаторы конференции выложили видеозапись.
Ниже перечислены некоторые типы непродуктивного поведения на код-ревью и рекомендации, как сделать процесс более доброжелательным и устранить токсичность. Все поведенческие модели проверены либо мной, либо в известной мне компании. В прошлом за мной тоже водились такие грешки.
Непродуктивное поведение № 1: передача мнения как факта
Не делайте утверждений, если не можете сослаться на документацию, формализованные рекомендации и примеры кода в поддержку этих утверждений. Люди должны знать, почему их просят внести изменения, а личные предпочтения другого разработчика не являются достаточно хорошим аргументом.
Вместо того, чтобы сказать:
Этот компонент следует сделать без изменения состояния (stateless).
…лучше предоставить некоторый контекст:
Поскольку у этого компонента нет методов жизненного цикла или состояния, его можно сделать stateless. Это повысит производительность и читаемость кода. *Вот* некоторая документация.
Передача мнения как факта часто встречается при обсуждении стиля и синтаксиса. Это действительно важные темы, но не для код-ревью, потому что стиль и синтаксис не имеют ничего общего с проблемой, которую изначально решает разработчик.
Такие обсуждения лучше вести отдельно и определиться со стилем для всей команды. Внедрите линтер и автоматические исправления кода. Затем будете ссылаться на эти рекомендации по стилю, а не на своё личное мнение.
Особенно важно не выдавать своё мнение за факт, если у вас более высокий ранг и авторитет в команде или компании. У разработчика создаётся впечатление, что у него нет выбора, кроме как послушно выполнить ваши требования.
Непродуктивное поведение № 2: лавина комментариев
Когда человек делает ошибку, то велики шансы, что эта ошибка встречается в нескольких местах. Я заметила, что рецензенты иногда указывают каждый из случаев вместо того, чтобы оставлять одну подробную заметку со ссылками на полезные ресурсы.
Консолидация комментариев позволяет передать одно и то же сообщение, не подавляя автора. Лавина дублированных комментариев по одной проблеме выглядит как придирки.
Непродуктивно и подавляюще:
![](https://habrastorage.org/getpro/habr/post_images/574/c8f/be9/574c8fbe93c02b6e3d70ee4be85431c1.png)
Несколько комментариев для одной повторяющейся ошибки (пробелы в конце строки)
Более полезный вариант:
![](https://habrastorage.org/getpro/habr/post_images/980/874/0a3/9808740a3535ebdbc9d1a54f6657155f.png)
Консолидированный фидбек
Я могу понять, что иногда полезно указать на каждое место ошибки, поскольку комментарий исчезает при исправлении в последующих коммитах. Но если ошибка встречается многократно, очевидно, что разработчик не знал об определённом руководстве, и ему просто следует указать на правильные ресурсы.
Непродуктивное поведение № 3: просить инженеров решать чужие проблемы, «пока они здесь»
Не просите разработчиков заниматься проблемами, не связанными напрямую с набором изменений или проблемой, которую пытается решить этот код. Даже если разработчик расширяет или изменяет грязный код с изобилием плохих практик, не просите его очистить и исправить этот чужой код.
Я не предлагаю лишить разработчиков ответственности за код только потому, что они изначально его не писали. На самом деле, нехорошо говорить что-то вроде «Чужой код — не моя проблема». Просто более целесообразно создать отдельный тикет и пулл-реквест для исправления грязного кода. Таким образом, вы избегаете резкого увеличения объёма чьей-то работы, решая проблему с техническим долгом.
TL;DR: Не просите разработчика исправлять проблемы «пока они здесь». Если код делает что требуется в тикете и не вводит новых проблем в кодовую базу, одобрите его, а затем создайте тикет для очистки кодовой базы.
Непродуктивное поведение № 4: задавать осуждающие вопросы
Избегайте задавать осуждающие вопросы вроде такого:
Почему ты здесь просто не сделал ____ ?
Здесь подразумевается, что должно быть очевидным некое простое решение. Это заставляет разработчика защищаться.
Часто эти осуждающие вопросы — просто завуалированные требования. Вместо этого предложите рекомендацию (с документацией и ссылками), опустив резкие слова.
Вы можете сделать ____, что имеет преимущество ____.
Непродуктивное поведение № 5: сарказм
В отзывах нет места сарказму. Как правило, саркастические комментарии не обеспечивают контекст и эффективную обратную связь. Вместо этого подробно опишите проблему и предоставьте рекомендации, но оставьте в стороне едкие шутки.
Непродуктивно:
Вы хотя бы тестировали этот код перед отправкой?
Полезно:
Происходит сбой при вводе отрицательного числа. Не могли бы вы этим заняться, пожалуйста?
Вот ещё один пример комментария в код-ревью, который ни смешон, ни полезен:
Мы не злобные, мы беспощадные. Как видите, я написал «бип!» — на импорте каждого файла, к которому вы прикасались. Я имел в виду следующее: «Ваш импорт нарушает нашу стандартную конвенцию, которая предполагает определённый порядок: сначала встроенные модули, затем сторонние, а затем уровень проекта», но это слишком длинная фраза, чтобы набирать её в каждом случае.
В приведённом примере «бип!» — совершенно не полезно и не содержательно. Это педантичный юмор, который не помогает автору.
Непродуктивное поведение № 6: Эмодзи вместо описания проблемы
При указании на проблемы в коде избегайте смайликов с большим пальцем вниз или эмодзи с тошнящим человечком. Это так же бесполезно, как и сарказм, по тем же причинам. Эмодзи загадочны, их легко неверно истолковать. Люди впустую тратят время, пытаясь понять ваши мысли.
![](https://habrastorage.org/getpro/habr/post_images/9d8/100/dfe/9d8100dfe4fdacbe8a0f8c4a3d6e4d68.png)
Не используйте эмодзи для указания на проблемы с кодированием
В любом случае у вас не должно быть эмоциональной реакции на ошибки программирования.
Вполне нормально использовать положительные смайлики, такие как «большой палец вверх» или «ура!», но не для указания на проблемы, а как отзыв на хороший код.
![](https://habrastorage.org/getpro/habr/post_images/643/524/76d/64352476d084ac094f8311415a7b9aba.png)
Одобряющие смайлики — это здорово
Непродуктивное поведение № 7: не отвечать на все комментарии
Авторы тоже вносят свой вклад в токсичность обсуждения. Если вы объединяете код, не ответив на все замечания, то это вызывает удивление у человека: он старался вам помочь, а вы даёте понять, что некоторые отзывы не имеют значения.
Если комментарий нерелевантен или вы не будете принимать меры, просто кратко объясните, почему.
Не игнорируйте коллег.
![](https://habrastorage.org/getpro/habr/post_images/76a/3e1/290/76a3e12901700a88a9e4fa089735c5c6.png)
Объединение кода без ответа на комментарий
Непродуктивное поведение № 8: игнорирование токсичного поведения, если оно исходит от лучших профессионалов
Токсичное поведение не следует игнорировать или недооценивать только по той причине, что разработчик является отличным профессионалом и чрезвычайно продуктивным сотрудником. Хотя он делает фантастическую работу, но важно иметь в виду, что его токсичное поведение приводит к стрессу и выгоранию других разработчиков.
Опыт работы с человеком, который демонстрирует токсичное поведение:
Другие сочтут, что работа с этим человеком истощает и демотивирует. Они пойдут на всё, чтобы избежать взаимодействия с ним, даже если это негативно повлияет на их способность выполнять задачи. Нарушатся коммуникации во всей организации, а сотрудники начнут искать вакансии в других местах. В то время как вы столкнётесь с последствиями утечки талантов и неудачными проектами, этот конкретный разработчик продолжит спокойно работать, как будто ничего не случилось. — Джозеф Гефро
Если не предпринять шагов по исправлению ситуации, возможны серьёзные последствия: разработчики будут перегружены, атакованы и демотивированы. Они начнут бояться фидбека, который на самом деле должен помогать им расти.
Я лично испытывала большое беспокойство всякий раз, когда получала электронное письмо с комментариями на мой пулл-реквест от одного бывшего коллеги (известного своими токсичными, малополезными отзывами). Хотя токсичное поведение на всех влияет по-разному, но определённо никто не выигрывает от этой токсичности.
Примечание: Хочу внести ясность, что если разработчик не удержался и однажды проявил такое поведение, это не делает его «токсичным». Речь идёт о многократных оскорблениях и едких комментариях.
Полезные практики код-ревью
Ниже приведены некоторые рекомендации, которые относятся к любому нормальному общению, хотя здесь мы говорим в контексте программирования и код-ревью.
Полезное поведение № 1: используйте вопросы или рекомендации для налаживания диалога
Никогда не требуйте от людей внесения правок и не задавайте осуждающих вопросов, потому что это препятствует диалогу между вами и собеседником.
Почему вы просто не объединили эти преобразования в файле с константами?
Формально это вопрос, но он не создаёт диалог между вами и собеседником. Он просто заставляет его защищаться. Вместо этого спросите, что собеседник думает о вашем решении, например:
Что вы думаете о том, чтобы вытащить эти преобразования в файл констант? Их довольно много, так что отдельный файл вполне может иметь смысл в данный момент.
…или предложите рекомендацию:
В этом файле у вас много вызовов трансляции для «функции x». Возможно, имеет смысл создать отдельный файл с её константами.
Полезное поведение № 2: сотрудничать, не прятаться
Когда вы программируете вместе, вы должны быть рядом, задавать вопросы, обсуждать и указывать на ресурсы.
«…когда вы хотите помочь или работать вместе, вы должны полностью участвовать, а не просто иногда вмешиваться» — руководство Recurse Center
Полезное поведение № 3: отвечайте на каждый комментарий
Если вы как автор не планируете применять советы ревьюера, просто сделайте запись, сообщив об этом. Не игнорируйте тех, кто тратит время, чтобы помочь вам.
Например:
Человек A: — Что вы думаете о создании вспомогательной функции для этого вызова API? В остальном всё хорошо.
Человек Б: — Эта строка не входила в мой набор изменений. Я объединю код, создам отдельный тикет на GitHub и запишу в план работ для нашей группы.
Полезное поведение № 4: знать, когда лучше организовать личную встречу
После десятков комментариев и предлагаемых решений должно быть ясно, что онлайн-общение стало непродуктивным для рассматриваемого вопроса. Разошлите всем вовлечённым коллегам приглашение на встречу.
Таким образом, группа сможет быстрее принять решение и применить его.
Проблемы, которые занимают часы и тонны комментариев, обычно можно решить за несколько минут продуктивного разговора. — «Аккуратная Java»
Полезное поведение № 5: Используйте возможности для обучения и не хвастайтесь
Прежде чем принять участие в обзоре кода, спросите себя:
Ваш комментарий действительно помогает учиться или вы придираетесь?
Подумайте о своём комментарии. Помните, что цель кода-ревью — научить и помочь другим разработчикам расти. Это не трибуна для выступлений.
Полезное поведение № 6: не показывать удивление
Будьте осторожны, чтобы не выказать удивление, если человек чего-то не знает. Не огорчайте людей тем, что они «должны уже знать» данную информацию. И наоборот, не стесняйтесь признать, что вам не хватает опыта по теме — это отличный способ попросить помощи.
Подробнее см. в правиле «Не притворяйтесь удивлённым» из руководства Recurse Center.
Полезное поведение № 7: автоматизируйте всё, что можно
Нет особого смысла обсуждать ошибки, которые могут выловить линтеры, хуки Git или автоматические тесты. Это тонна лишних комментариев, которые выглядят как придирки. Люди не слишком хороши в выявлении таких ошибок, поэтому и созданы инструменты автоматизации.
Есть также инструменты для автоматического запуска тестов при добавлении нового кода. Они отображают предупреждения, если набор изменений нарушает какой-то из тестов. Например, TeamCity и Jenkins CI.
Кроме того, используйте хуки Git для запуска тестов и линтеров на новом коде. Они перехватят коммит, если обнаружат баги.
Пусть на проблемы указывает инструмент, чтобы это не приходилось делать людям.
Полезное поведение № 8: не перенимайте токсичное поведение
Не нужно перенимать токсичное поведение в своих код-ревью, потому что это похоже на месть или некий ритуал дедовщины для новых разработчиков.
Найдите коллег, которые вас поддержат.
Если вы заметили непродуктивные код-ревью, попытайтесь сообщить коллеге, говорите прямо и честно (если чувствуете себя безопасно в своей должности/компании).
Полезное поведение № 9: менеджеры, внимательно рассматривайте кандидатов, прислушивайтесь к команде и применяйте свои полномочия
У менеджеров большие возможности для создания позитивной и благоприятной атмосферы в коллективе.
Перефразируем советы из статьи «Вред токсичных разработчиков»:
- Предотвратите появление токсичных разработчиков в команде. При найме обращайте внимание не только на техническую подготовку. Узнайте, как кандидат сотрудничает и общается. Критически проанализируйте его работу и посмотрите, как он реагирует. Убедитесь, что каждый кандидат улучшает культуру компании, а не просто вписывается в неё.
- Когда токсичный разработчик всё-таки попал в штат или достался по наследству, спросите мнение о нём у каждого сотрудника в личном разговоре. Если разработчик токсичный, это всплывёт в отзывах о нём.
- Поговорите с токсичным разработчиком. Приведите конкретные примеры эффективных комментариев в код-ревью. Непрерывно работайте с ним, продолжая собирать информацию в личных разговорах с его коллегами и менеджером.
- Не изолируйте токсичного разработчика. (*)
- Повторяйте сотрудникам о необходимости доброжелательного окружения.
(*) Хотя в статье предлагается изолировать токсичных разработчиков, я считаю важным поощрить токсичного разработчика к сотрудничеству с командой, но более здоровыми способами. Изоляция не решит проблему. Если дать ему отдельную работу, токсичность снизится в краткосрочной перспективе, но разработчик не откажется от своего непродуктивного поведения. Он только потеряет трибуну, чтобы его демонстрировать.
Полезное поведение № 10: установите стандарт, пока команда мала и растёт
Небольшие команды способны принять новые идеи и реализовать их. Даже если вы не считаете необходимым устанавливать стандарты, потому что сейчас проблем нет, но важно сохранить хорошие практики сотрудничества, когда придёт пополнение.
Полезное поведение № 11: понять, что вы можете быть частью проблемы
Чтобы создать более благоприятную среду, важно быть честным с собой и размышлять над любым неэффективным поведением.
Будучи джуниором, я несколько раз замечала ошибку в чьём-то коде и радовалась, ведь я могла оставить комментарий! Теперь я понимаю, что использовала такую возможность, чтобы хвастаться, а не помогать. Нужно внимательно оценивать свои действия.
Думаю, что каждому полезно уделить время этому трудному самоанализу.
И последнее…
Мы говорим не о содержании отзывов, а только о тоне
Знаю, что отзывы важны, и мы тут не воюем против них. Я не прошу ставить под угрозу качество кода. Доброжелательная культура код-ревью и высокое качество кода не должны быть взаимоисключающими.
Просто надеюсь, что люди предпримут шаги, чтобы обеспечить конструктивную, действенную обратную связь и создать более благоприятные условия, в которой разработчики будут чувствовать себя комфортно, будут учиться, расти и не бояться совершать ошибки. Все мы можем совершенствоваться вместе.