Хабрахабр

[Перевод] Отучаемся от токсичных практик на код-ревью

Код-ревью частенько порождают споры. При подготовке лекции «Отучаемся от токсичного поведения на код-ревью» на конференции 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: понять, что вы можете быть частью проблемы

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

Теперь я понимаю, что использовала такую возможность, чтобы хвастаться, а не помогать. Будучи джуниором, я несколько раз замечала ошибку в чьём-то коде и радовалась, ведь я могла оставить комментарий! Нужно внимательно оценивать свои действия.

Думаю, что каждому полезно уделить время этому трудному самоанализу.

И последнее…

Мы говорим не о содержании отзывов, а только о тоне

Знаю, что отзывы важны, и мы тут не воюем против них. Я не прошу ставить под угрозу качество кода. Доброжелательная культура код-ревью и высокое качество кода не должны быть взаимоисключающими.

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

Теги
Показать больше

Похожие статьи

Добавить комментарий

Ваш e-mail не будет опубликован. Обязательные поля помечены *

Кнопка «Наверх»
Закрыть