Код-ревью частенько порождают споры. При подготовке лекции «Отучаемся от токсичного поведения на код-ревью» на конференции AlterConf я была готова услышать кучу возражений и критики. Но совершенно не ожидала, что сообщество настолько поддержит идею. Я предполагала сопротивление, но сообщество очень доброжелательно и с одобрением приняло меня.
Меня попросили поделиться слайдами, но теперь я подумала, что слайды сами по себе малополезны и вырваны из контекста: им не хватает объяснений. Поэтому решила опубликовать эту статью. Позже организаторы конференции выложили видеозапись.
Ниже перечислены некоторые типы непродуктивного поведения на код-ревью и рекомендации, как сделать процесс более доброжелательным и устранить токсичность. Все поведенческие модели проверены либо мной, либо в известной мне компании. В прошлом за мной тоже водились такие грешки.
Непродуктивное поведение № 1: передача мнения как факта
Не делайте утверждений, если не можете сослаться на документацию, формализованные рекомендации и примеры кода в поддержку этих утверждений. Люди должны знать, почему их просят внести изменения, а личные предпочтения другого разработчика не являются достаточно хорошим аргументом.
Вместо того, чтобы сказать:
Этот компонент следует сделать без изменения состояния (stateless).
…лучше предоставить некоторый контекст:
Поскольку у этого компонента нет методов жизненного цикла или состояния, его можно сделать stateless. Это повысит производительность и читаемость кода. *Вот* некоторая документация.
Передача мнения как факта часто встречается при обсуждении стиля и синтаксиса. Это действительно важные темы, но не для код-ревью, потому что стиль и синтаксис не имеют ничего общего с проблемой, которую изначально решает разработчик.
Такие обсуждения лучше вести отдельно и определиться со стилем для всей команды. Внедрите линтер и автоматические исправления кода. Затем будете ссылаться на эти рекомендации по стилю, а не на своё личное мнение.
Особенно важно не выдавать своё мнение за факт, если у вас более высокий ранг и авторитет в команде или компании. У разработчика создаётся впечатление, что у него нет выбора, кроме как послушно выполнить ваши требования.
Непродуктивное поведение № 2: лавина комментариев
Когда человек делает ошибку, то велики шансы, что эта ошибка встречается в нескольких местах. Я заметила, что рецензенты иногда указывают каждый из случаев вместо того, чтобы оставлять одну подробную заметку со ссылками на полезные ресурсы.
Консолидация комментариев позволяет передать одно и то же сообщение, не подавляя автора. Лавина дублированных комментариев по одной проблеме выглядит как придирки.
Непродуктивно и подавляюще:
Несколько комментариев для одной повторяющейся ошибки (пробелы в конце строки)
Более полезный вариант:
Консолидированный фидбек
Я могу понять, что иногда полезно указать на каждое место ошибки, поскольку комментарий исчезает при исправлении в последующих коммитах. Но если ошибка встречается многократно, очевидно, что разработчик не знал об определённом руководстве, и ему просто следует указать на правильные ресурсы.
Непродуктивное поведение № 3: просить инженеров решать чужие проблемы, «пока они здесь»
Не просите разработчиков заниматься проблемами, не связанными напрямую с набором изменений или проблемой, которую пытается решить этот код. Даже если разработчик расширяет или изменяет грязный код с изобилием плохих практик, не просите его очистить и исправить этот чужой код.
Я не предлагаю лишить разработчиков ответственности за код только потому, что они изначально его не писали. На самом деле, нехорошо говорить что-то вроде «Чужой код — не моя проблема». Просто более целесообразно создать отдельный тикет и пулл-реквест для исправления грязного кода. Таким образом, вы избегаете резкого увеличения объёма чьей-то работы, решая проблему с техническим долгом.
TL;DR: Не просите разработчика исправлять проблемы «пока они здесь». Если код делает что требуется в тикете и не вводит новых проблем в кодовую базу, одобрите его, а затем создайте тикет для очистки кодовой базы.
Непродуктивное поведение № 4: задавать осуждающие вопросы
Избегайте задавать осуждающие вопросы вроде такого:
Почему ты здесь просто не сделал ____ ?
Здесь подразумевается, что должно быть очевидным некое простое решение. Это заставляет разработчика защищаться.
Часто эти осуждающие вопросы — просто завуалированные требования. Вместо этого предложите рекомендацию (с документацией и ссылками), опустив резкие слова.
Вы можете сделать ____, что имеет преимущество ____.
Непродуктивное поведение № 5: сарказм
В отзывах нет места сарказму. Как правило, саркастические комментарии не обеспечивают контекст и эффективную обратную связь. Вместо этого подробно опишите проблему и предоставьте рекомендации, но оставьте в стороне едкие шутки.
Непродуктивно:
Вы хотя бы тестировали этот код перед отправкой?
Полезно:
Происходит сбой при вводе отрицательного числа. Не могли бы вы этим заняться, пожалуйста?
Вот ещё один пример комментария в код-ревью, который ни смешон, ни полезен:
Мы не злобные, мы беспощадные. Как видите, я написал «бип!» — на импорте каждого файла, к которому вы прикасались. Я имел в виду следующее: «Ваш импорт нарушает нашу стандартную конвенцию, которая предполагает определённый порядок: сначала встроенные модули, затем сторонние, а затем уровень проекта», но это слишком длинная фраза, чтобы набирать её в каждом случае.
В приведённом примере «бип!» — совершенно не полезно и не содержательно. Это педантичный юмор, который не помогает автору.
Непродуктивное поведение № 6: Эмодзи вместо описания проблемы
При указании на проблемы в коде избегайте смайликов с большим пальцем вниз или эмодзи с тошнящим человечком. Это так же бесполезно, как и сарказм, по тем же причинам. Эмодзи загадочны, их легко неверно истолковать. Люди впустую тратят время, пытаясь понять ваши мысли.
Не используйте эмодзи для указания на проблемы с кодированием
В любом случае у вас не должно быть эмоциональной реакции на ошибки программирования.
Вполне нормально использовать положительные смайлики, такие как «большой палец вверх» или «ура!», но не для указания на проблемы, а как отзыв на хороший код.
Одобряющие смайлики — это здорово
Непродуктивное поведение № 7: не отвечать на все комментарии
Авторы тоже вносят свой вклад в токсичность обсуждения. Если вы объединяете код, не ответив на все замечания, то это вызывает удивление у человека: он старался вам помочь, а вы даёте понять, что некоторые отзывы не имеют значения.
Если комментарий нерелевантен или вы не будете принимать меры, просто кратко объясните, почему.
Не игнорируйте коллег.
Объединение кода без ответа на комментарий
Непродуктивное поведение № 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: понять, что вы можете быть частью проблемы
Чтобы создать более благоприятную среду, важно быть честным с собой и размышлять над любым неэффективным поведением.
Будучи джуниором, я несколько раз замечала ошибку в чьём-то коде и радовалась, ведь я могла оставить комментарий! Теперь я понимаю, что использовала такую возможность, чтобы хвастаться, а не помогать. Нужно внимательно оценивать свои действия.
Думаю, что каждому полезно уделить время этому трудному самоанализу.
И последнее…
Мы говорим не о содержании отзывов, а только о тоне
Знаю, что отзывы важны, и мы тут не воюем против них. Я не прошу ставить под угрозу качество кода. Доброжелательная культура код-ревью и высокое качество кода не должны быть взаимоисключающими.
Просто надеюсь, что люди предпримут шаги, чтобы обеспечить конструктивную, действенную обратную связь и создать более благоприятные условия, в которой разработчики будут чувствовать себя комфортно, будут учиться, расти и не бояться совершать ошибки. Все мы можем совершенствоваться вместе.
