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

в 9:23, , рубрики: Jenkins CI, teamcity, критика, общение, Программирование, тестирование, управление разработкой
Отучаемся от токсичных практик на код-ревью - 1

Код-ревью частенько порождают споры. При подготовке лекции «Отучаемся от токсичного поведения на код-ревью» на конференции AlterConf я была готова услышать кучу возражений и критики. Но совершенно не ожидала, что сообщество настолько поддержит идею. Я предполагала сопротивление, но сообщество очень доброжелательно и с одобрением приняло меня. 

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

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

Непродуктивное поведение № 1: передача мнения как факта

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

Вместо того, чтобы сказать:

Этот компонент следует сделать без изменения состояния (stateless).

…лучше предоставить некоторый контекст:

Поскольку у этого компонента нет методов жизненного цикла или состояния, его можно сделать stateless. Это повысит производительность и читаемость кода. *Вот* некоторая документация.

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

Такие обсуждения лучше вести отдельно и определиться со стилем для всей команды. Внедрите линтер и автоматические исправления кода. Затем будете ссылаться на эти рекомендации по стилю, а не на своё личное мнение.

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

Непродуктивное поведение № 2: лавина комментариев

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

Консолидация комментариев позволяет передать одно и то же сообщение, не подавляя автора. Лавина дублированных комментариев по одной проблеме выглядит как придирки.

Непродуктивно и подавляюще:

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

Несколько комментариев для одной повторяющейся ошибки (пробелы в конце строки)

Более полезный вариант:

Отучаемся от токсичных практик на код-ревью - 3
Консолидированный фидбек

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

Непродуктивное поведение № 3: просить инженеров решать чужие проблемы, «пока они здесь»

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

Я не предлагаю лишить разработчиков ответственности за код только потому, что они изначально его не писали. На самом деле, нехорошо говорить что-то вроде «Чужой код — не моя проблема». Просто более целесообразно создать отдельный тикет и пулл-реквест для исправления грязного кода. Таким образом, вы избегаете резкого увеличения объёма чьей-то работы, решая проблему с техническим долгом.

TL;DR: Не просите разработчика исправлять проблемы «пока они здесь». Если код делает что требуется в тикете и не вводит новых проблем в кодовую базу, одобрите его, а затем создайте билет для очистки кодовой базы.

Непродуктивное поведение № 4: задавать осуждающие вопросы

Избегайте задавать осуждающие вопросы вроде такого:

Почему ты здесь просто не сделал ____ ?

Здесь подразумевается, что должно быть очевидным некое простое решение. Это заставляет разработчика защищаться.

Часто эти осуждающие вопросы — просто завуалированные требования. Вместо этого предложите рекомендацию (с документацией и ссылками), опустив резкие слова.

Вы можете сделать ____, что имеет преимущество ____.

Непродуктивное поведение № 5: сарказм

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

Непродуктивно:

Вы хотя бы тестировали этот код перед отправкой?

Полезно:

Происходит сбой при вводе отрицательного числа. Не могли бы вы этим заняться, если не сложно?

Вот ещё один пример комментария в код-ревью, который ни смешон, ни полезен:

Мы не злобные, мы беспощадные. Как видите, я написал «бип!» — на импорте каждого файла, к которому вы прикасались. Я имел в виду следующее: «Ваш импорт нарушает нашу стандартную конвенцию, которая предполагает определённый порядок: сначала встроенные модули, затем сторонние, а затем уровень проекта», но это слишком длинная фраза, чтобы набирать её в каждом случае.

В приведённом примере «бип!» — совершенно не полезно и не содержательно. Это педантичный юмор, который не помогает автору.

Непродуктивное поведение № 6: Эмодзи вместо описания проблемы

При указании на проблемы в коде избегайте смайликов с большим пальцем вниз или эмодзи с тошнящим человечком. Это так же бесполезно, как и сарказм, по тем же причинам. Эмодзи загадочны, их легко неверно истолковать. Люди впустую тратят время, пытаясь понять ваши мысли.

Отучаемся от токсичных практик на код-ревью - 4
Не используйте эмодзи для указания на проблемы с кодированием

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

Вполне нормально использовать положительные смайлики, такие как «большой палец вверх» или «ура!», но не для указания на проблемы, а как отзыв на хороший код.

Отучаемся от токсичных практик на код-ревью - 5
Одобряющие смайлики — это здорово

Непродуктивное поведение № 7: не отвечать на все комментарии

Авторы тоже вносят свой вклад в токсичность обсуждения. Если вы объединяете код, не ответив на все замечания, то это вызывает удивление у человека: он старался вам помочь, а вы даёте понять, что некоторые отзывы не имеют значения.

Если комментарий нерелевантен или вы не будете принимать меры, просто кратко объясните, почему.

Не игнорируйте коллег.

Отучаемся от токсичных практик на код-ревью - 6
Объединение кода без ответа на комментарий

Непродуктивное поведение № 8: игнорирование токсичного поведения, если оно исходит от лучших профессионалов

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

Опыт работы с человеком, который демонстрирует токсичное поведение:

Другие сочтут, что работа с этим человеком истощает и демотивирует. Они пойдут на всё, чтобы избежать взаимодействия с ним, даже если это негативно повлияет на их способность выполнять задачи. Нарушатся коммуникации во всей организации, а сотрудники начнут искать вакансии в других местах. В то время как вы столкнётесь с последствиями утечки талантов и неудачными проектами, этот конкретный разработчик продолжит спокойно работать, как будто ничего не случилось. — Джозеф Гефро

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

Я лично испытывала большое беспокойство всякий раз, когда получала электронное письмо с комментариями на мой пулл-реквест от одного бывшего коллеги (известного своими токсичными, малополезными отзывами). Хотя токсичное поведение на всех влияет по-разному, но определённо никто не выигрывает от этой токсичности.

Примечание: Хочу внести ясность, что если разработчик не удержался и однажды проявил такое поведение, это не делает его «токсичным». Речь идёт о многократных оскорблениях и едких комментариях.

Полезные практики код-ревью

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

Полезное поведение № 1: используйте вопросы или рекомендации для налаживания диалога

Никогда не требуйте от людей внесения правок и не задавайте осуждающих вопросов, потому что это препятствует диалогу между вами и собеседником.

Почему вы просто не объединили эти преобразования в файле с константами?

Формально это вопрос, но он не создаёт диалог между вами и собеседником. Он просто заставляет его защищаться. Вместо этого спросите, что собеседник думает о вашем решении, например:

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

…или предложите рекомендацию:

В этом файле у вас много вызовов трансляции для «функции x». Возможно, имеет смысл создать отдельный файл с её константами.

Полезное поведение № 2: сотрудничать, не прятаться

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

«…когда вы хотите помочь или работать вместе, вы должны полностью участвовать, а не просто иногда вмешиваться» — руководство Recurse Center

Полезное поведение № 3: отвечайте на каждый комментарий

Если вы как автор не планируете применять советы ревьюера, просто сделайте запись, сообщив об этом. Не игнорируйте тех, кто тратит время, чтобы помочь вам.

Например:

Человек A: — Что вы думаете о создании вспомогательной функции для этого вызова API? В остальном всё хорошо.

Человек Б: — Эта строка не входила в мой набор изменений. Я объединю код, создам отдельный тикет на GitHub и запишу в план работ для нашей группы.

Полезное поведение № 4: знать, когда лучше организовать личную встречу

После десятков комментариев и предлагаемых решений должно быть ясно, что онлайн-общение стало непродуктивным для рассматриваемого вопроса. Разошлите всем вовлечённым коллегам приглашение на встречу.

Таким образом, группа сможет быстрее принять решение и применить его.

Проблемы, которые занимают часы и тонны комментариев, обычно можно решить за несколько минут продуктивного разговора. — «Аккуратная Java»

Полезное поведение № 5: Используйте возможности для обучения и не хвастайтесь

Прежде чем принять участие в обзоре кода, спросите себя:

Ваш комментарий действительно помогает учиться или вы придираетесь?

Подумайте о своём комментария. Помните, что цель кода-ревью — научить и помочь другим разработчикам расти. Это не в трибуна для выступлений.

Полезное поведение № 6: не показывать удивление

Будьте осторожны, чтобы не выказать удивление, если человек чего-то не знает. Не огорчайте людей тем, что они «должны уже знать» данную информацию. И наоборот, не стесняйтесь признать, что вам не хватает опыта по теме — это отличный способ попросить помощи.

Подробнее см. в правиле «Не притворяйтесь удивлённым» из руководства Recurse Center.

Полезное поведение № 7: автоматизируйте всё, что можно

Нет особого смысла обсуждать ошибки, которые могут выловить линтеры, хуки Git или автоматические тесты. Это тонна лишних комментариев, которые выглядят как придирки. Люди не слишком хороши в выявлении таких ошибок, поэтому и созданы инструменты автоматизации.

Есть также инструменты для автоматического запуска тестов при добавлении нового кода. Они отображают предупреждения, если набор изменений нарушает какой-то из тестов. Например, TeamCity и Jenkins CI.

Кроме того, используйте хуки Gi для запуска тестов и линтеров на новом коде. Они перехватят коммит, если обнаружат баги.

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

Полезное поведение № 8: не перенимайте токсичное поведение

Не нужно перенимать токсичное поведение в своих код-ревью, потому что это похоже на месть или некий ритуал дедовщины для новых разработчиков.

Найдите коллег, которые вас поддержат.

Если вы заметили непродуктивные код-ревью, попытайтесь сообщить коллеге, говорите прямо и честно (если чувствуете себя безопасно в своей должности/компании).

Полезное поведение № 9: менеджеры, внимательно рассматривайте кандидатов, прислушивайтесь к команде и применяйте свои полномочия

У менеджеров большие возможности для создания позитивной и благоприятной атмосферы в коллективе.

Перефразируем советы из статьи «Вред токсичных разработчиков»:

  • Предотвратите появление токсичных разработчиков в команде. При найме обращайте внимание не только на техническую подготовку. Узнайте, как кандидат сотрудничает и общается. Критически проанализируйте его работу и посмотрите, как он реагирует. Убедитесь, что каждый кандидат улучшает культуру компании, а не просто вписывается в неё.
  • Когда токсичный разработчик всё-таки попал в штат или достался по наследству, спросите мнение о нём у каждого сотрудника в личном разговоре. Если разработчик токсичный, это всплывёт в отзывах о нём.
  • Поговорите с токсичным разработчиком. Приведите конкретные примеры эффективных комментариев в код-ревью. Непрерывно работайте с ним, продолжая собирать информацию в личных разговорах с его коллегами и менеджером.
  • Не изолируйте токсичного разработчика. (*)
  • Повторяйте сотрудникам о необходимости доброжелательного окружения.

(*) Хотя в статье предлагается изолировать токсичных разработчиков, я считаю важным поощрить токсичного разработчика к сотрудничеству с командой, но более здоровыми способами. Изоляция не решит проблему. Если дать ему отдельную работу, токсичность снизится в краткосрочной перспективе, но разработчик не откажется от своего непродуктивного поведения. Они только потеряет трибуну, чтобы его демонстрировать.

Полезное поведение № 10: установите стандарт, пока команда мала и растёт

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

Полезное поведение № 11: понять, что вы можете быть частью проблемы

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

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

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

И последнее…

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

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

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

Автор: m1rko

Источник


* - обязательные к заполнению поля


https://ajax.googleapis.com/ajax/libs/jquery/3.4.1/jquery.min.js