[Перевод] Ревью кода в распределенной команде
Здесь описаны мои исследования, как сделать ревизию кода в команде более приятным занятием, которое может дать новый опыт всем участникам. У нас полностью географически распределённая команда, все коммуникации выполняются через интернет, и зачастую асинхронно. Мы используем Trello для описания возможностей продуктов, поодиночке создаём код, отправляем в GitHub пулл-реквесты, а также пользуемся встроенной в GitHub функцией их ревью. Это отличается от просмотра кода лицом к лицу в офисе и даже по видеочату.
Если не подходить к делу всерьёз, то асинхронная и письменная ревизия кода может стать причиной катастрофы в команде, приведя к ухудшению взаимодействия и сотрудничества. Но если все участники будут стараться делать всё хорошо, то такой подход может работать очень эффективно.
Прежде чем начать рассуждать о том, как делать и как не делать ревью, полезно подумать о том, для кого в компании оно нужно, и в соответствии с этим вырабатывать требования. И хорошо бы не забывать, что ревью — это не просто поиск багов и проблем в структуре кода.
Ревью нужно для улучшения качества кода и морального духа в команде.
Анализ кода друг друга — один из главных путей взаимодействия между разработчиками во время обычного рабочего дня. Это должен быть вдохновляющий процесс, чтобы все участники ждали его и активно участвовали в нём.
Вот основные задачи, решаемые с помощью ревизий:
- Создание внутренней культуры команды. Ревью — это один из основных процессов, который подразумевает наличие командной культуры. И если участникам неприятно участвовать в ревью, значит, внятной командной культуры нет. Ревизии должны создавать позитивную атмосферу, терпимость и дружественность.
- Улучшение рабочих взаимоотношений благодаря возможности чаще разговаривать с коллегами.
- Снятие напряжённости в отношениях с помощью положительных отзывов о чужом коде.
- Обучение. Проводящий ревью может узнать для себя новые вещи из чужого кода, а автор — из отзыва.
- Выход за границы своей зоны ответственности, позволяющий всем участникам команды анализировать разные части кода, ознакомляясь со всей кодовой базой.
- Наставничество и образование. Ревью — это возможность для более опытных участников стать наставниками и обучать других. Но имейте в виду, что, поскольку время уже прошло и код написан, ревью — не всегда лучшее время для обучения. Разработчики должны совместно работать над техническим проектом как до, так и во время создания кода.
- Качество кода. И наконец, качество (включая баги, удобство в сопровождении, документацию, организацию, архитектуру, удобство использования продукта…).
Что не надо делать при ревью
Возможно, эта часть покажется вам довольно негативной, но зато после неё идёт часть с приятными вещами!
Есть много разных способов сделать ревью кода неудачным. Воинственные, злые и непредсказуемые комментарии способны лишить команду удовольствия от работы и превратить ревизию в эмоционально тяжёлую обязанность.
Понимание, чего следует избегать при ревью, как раз и отличает ценную часть работы команды от жестокого и неприятного наказания. Эрик Дитрих, «Чего нужно избегать при ревью кода»
Не проводите ревью формально и с минимальными комментариями
(a.k.a. узнать, насколько плохо думают о вас коллеги)
Не нужно всего лишь указывать на ошибки, ничего к этому не добавляя.
Разработчики тратят много времени на код и гордятся им, и ревью — их шанс продемонстрировать свою работу. А при формально-минималистичном подходе всё, на что вы можете надеяться, — это комментарии в стиле «Выглядит неплохо, смерджил». Такое ревью достойно названия «Узнай, насколько плохо о тебе думают».
Невнятное и непродуманное общение может привести к долговременным неприятным последствиям. Командная культура испортится, что снизит продуктивность. При этом разработчики часто получают отзывы вроде «похоже, эта порка придумана специально для того, чтобы выбить из них дух», а сами ревизии превращаются в «ментальные соревнования, в которых люди стреляют по цели…, а цель — это разработчик, написавший код».
Не думайте, что достаточно лишь «критиковать код, а не кодера»
Это один из самых популярных советов: критикуйте код, а не кодера. Он хорош (если вы критикуете коллегу, а не его код, то у вас проблемы). Но этого недостаточно. Написание кода — творческий процесс, в который разработчик вкладывает сердце и душу. И если вы жестоко и тупо критикуете чей-то код, то тем самым вы критикуете самого человека. Нужно уметь критиковать. Найдите более позитивные способы анализа и вносите свои предложения, как улучшить код.
Следите за тоном
Не переходите на личности. Фразы вроде «Почему ты сделал именно так?» воспринимаются как нападки. Плохой вариант: «То, как ты написал эту функцию, затрудняет её чтение, добавь больше комментариев» (личное обращение подразумевает, что человек что-то сделал не так). Лучше скажите: «Тебе не кажется, что стоит добавить в код дополнительные комментарии, это облегчит чтение функции?» (звучит гораздо корректнее и позволяет сосредоточиться на улучшении кода).
Обойдитесь без требовательных или вызывающих выражений. Избегайте фраз, смысл которых заключается в «ты неправ». Не говорите людям, что они неправы или что они сказали/сделали что-то не то, им будет неприятно. В таких случаях люди могут уходить в защиту, перестают долго и творчески работать и учиться. Избегайте фраз наподобие:
- «Так не сработает»
- «Совершенно неправильно»
- «Почему ты просто не… ?»
Не сгущайте краски. Вы же не хотите, чтобы на критику обижались за необоснованность или преувеличенность? Не говорите «всегда», «никогда», «бесконечно», «ничего».
Не надо оскорблений. Избегайте ругательств вроде «дурак» или «идиот», даже если лично вы не считаете их обидными. У подобных слов есть определённый спектр значений, и он точно не улучшит настроение автора кода.
Не используйте нетерпеливых или пассивно-агрессивных оборотов. Всегда будьте сдержанны и дружелюбны, избегайте неприятных фраз, демонстрирующих раздражение и заставляющих людей чувствовать, что они плохо справляются:
- «Ещё раз повторяю: здесь нужно…»
- «Как я уже говорил…»
Не подливайте масла в огонь. Человеку может быть неприятно получить от кого-то комментарий с критикой, а под ним ещё несколько с »+1» или нравоучениями по тому же поводу. К тому же многочисленная критика способна просто запутать.
Не будьте непрошеным советчиком
Сдерживайте себя и не предлагайте внести в код все изменения, которые вы хотели бы внести. Вы можете деморализовать автора кода, предложив поменять столько, что придётся всё переписать. Так что выбирайте только самое важное и лучшее.
Одна из задач ревью — найти и исправить баги и проблемы в структуре кода. А вовсе не заставить автора написать код так, как это сделали бы вы. Не забывайте, что в программном обеспечении многие вещи можно решить по-разному, в зависимости от вкусов разработчика, и у каждого подхода есть свои плюсы и минусы. Собираясь предложить очередную правку, спросите себя: может быть, это просто моё мнение? Если автор выбрал другие способы, обосновав свой выбор, то лучше поддержите его. Уважайте право автора на собственные решения.
Даже если кто-то написал код не так, как это сделали бы вы, то это ещё не означает, что код написан неправильно. Главное — качественный, удобный в сопровождении код. Если он удовлетворяет этим критериям и принятым в команде правилам, то этого достаточно. Роберт Бог, «Эффективные ревью без боли»
Вы никогда не сможете заставить других написать именно такой код, какой написали бы вы, не надо и пытаться (если вам это так важно, то лучше сразу напишите код сами). Эрик Дитрих, «Чего нужно избегать при ревью кода»
Не нарушайте правило «Никаких сюрпризов»
Избегайте неожиданностей. Создайте для команды общее задокументированное соглашение о pull request«ах и стандартах кода и во время ревью опирайтесь на него, а не на своё мнение.
Не говорите только для того, чтобы услышать свой голос
Прежде чем написать отзыв, подумайте, чего вы хотите этим достичь. Все ли ваши комментарии необходимы? Помните, что вы пытаетесь конструктивно помочь, а не просто высказать свою точку зрения. Прежде чем опубликовать отзыв, перечитайте свои комментарии и спросите себя, все ли они действительно полезны и важны? Лишние удалите. Лучше анализировать свои заметки после просмотра кода, постфактум. Пока вы разбираетесь с кодом, пишите сколько угодно комментариев, а потом спокойно просмотрите их и решите, что оставить.
Оставлять комментарии только для того, чтобы подчеркнуть свою правоту, не всегда полезно (или разумно). Иногда лучше держать своё мнение при себе, чтобы сохранить длительные хорошие отношения с человеком. Кэтрин Дэниелс, «Как давать и получать отзывы»
Вы не обязаны находить проблемы в ходе каждого ревью
Если код хорош и может быть влит в кодовую базу без изменений, то всё прекрасно! Что надо делать при ревью
Ревью можно улучшить. Как именно? Ниже вы прочтёте предложения, собранные в интернете.
Хвалите хороший код
Не жалейте времени на то, чтобы похвалить сильные стороны кода, прежде чем указывать на недостатки и вносить предложения.
Человеческая природа такова, что нам нужно знать о своих успехах, а не только о неудачах. Разработка — это творческий процесс, в который люди вкладывают душу, они часто принимают многое близко к сердцу. Поэтому похвалы для них ещё более важны. Роберт Бог, «Эффективные ревью без боли»
Вы же хотите, чтобы участники команды воспринимали ревью как приятный и полезный опыт, а не как чистое критиканство. Положительные отзывы снижают напряжённость в отношениях и помогают людям лучше реагировать на критику.
Важно писать позитивные комментарии так, чтобы это не выглядело слащаво или фальшиво. Не должно создаваться впечатление, что вы просто стараетесь подсластить пилюлю.
Избегайте сомнительных комплиментов. Фразы вроде «Хорошая работа, но…» (дальше — список изменений, которые вы предлагаете сделать) выглядят наигранными.
Как сделать похвалы более честными?
- Оцените большинство хороших частей кода и другие аспекты и постарайтесь сказать, почему они вам понравились.
- Вникая в чужой код, оставляйте в комментариях свой «монолог на ходу». «Ага, понял, что это делает… Хорошо, подключается сюда и вызывает это, замечательно…, а эта часть зависит от тех двух, отлично». Ход мыслей другого программиста, который разбирается в твоём коде, — ещё одна форма проверки работы. А когда тебе попадаются части кода, которые ты не понимаешь, можно попросить объяснить.
Сначала — положительные моменты
Сразу задайте настроение, в первую очередь рассказав о том, что вам понравилось. Теперь это легко можно сделать благодаря новым возможностям ревью кода на GitHub, создавая многострочные комментарии и публикуя их скопом (вместо публикации по мере сохранения), а также делая краткое резюме (отображается в начале отзыва) до внесения комментариев:
Избегайте неизбежных обвинений
Спрашивайте, а не говорите. Лучше задавать вопросы, а не делать заявления.
Заявление — это обвинение. Фраза «Здесь ты не соблюдал стандарты» — это обвинение, намеренное или нет. Задайте вопрос: «Почему ты использовал здесь такой подход?» Роберт Бог, «Эффективные ревью без боли»
Если задавать вопросы, то это улучшит общий настрой, восприятие ревью изменится благодаря приглашению к диалогу и обучению, а автор кода с удовольствием объяснит причину или поищет более удачный способ решения.
В идеале в ответ на корректный вопрос автор сам найдёт баг или предложит внести в код улучшения.
Если вы видите какие-то моменты, которые могут быть ошибками, не надо сразу говорить людям, что они неправы. Обычно достаточно спросить: «Что будет, если я передам этому методу ноль?», и человек наверняка сам скажет: «У меня были сомнения, исправлю». Позволить ему самому решить проблему и предложить улучшить код — гораздо лучше, чем давать указание по исправлению несовершенств. Эрик Дитрих, «Используйте ревью кода для казни»
Плохо:
- «Здесь ты не соблюдал стандарты»
- «А — неправильно, надо использовать В»
- «Запутанный код»
- «Ты не инициализировал эти переменные»
Хорошо:
- «Почем ты выбрал здесь такое решение?»
- «Почему ты использовал A вместо B?»
- «Эта часть мне непонятна. Можешь объяснить?»
- «Я не нашёл, где инициализируются эти переменные»
Избегайте обвинительного «почему». Как и утверждения, вопросы «почему» тоже могут звучать обвинительно, так что без них будет лучше.
Плохо:
- «Почему здесь ты не следовал стандартам?»
- «Почему ты просто не…?»
Хорошо:
- «По какой причине ты решил отойти здесь от стандартов?»
- «Чем ты руководствовался, когда …?»
Будьте скромнее
Задавайте вопросы, а не требуйте. Вместо того чтобы сказать автору, какие нужны изменения, задайте ему вопрос о коде или внесите предложение и спросите автора, что он думает по этому поводу. Так вы передаёте мяч на его сторону поля и выказываете уважение его свободе воли, давая автору шанс объяснить своё решение или высказаться, считает ли ваше предложение полезным для кода.
Вместо «Давай назовём эту переменную username, потому что иначе непонятно» лучше перефразировать предложение в виде вопроса: «Может быть, назвать её username? Текущее имя выглядит не слишком понятно для меня, потому что оно также используется в другом контексте в someotherfile.js.». Дэниел Бадер,»7 способов избежать ухудшения отношений при ревью кода»
Когда вы что-то предлагаете, обязательно объясняйте причину, по которой это изменение может улучшить код.
Используйте личные примеры. Покажите автору кода, что не только он совершал такую ошибку. Это отличный способ смягчить критику: «Мне самому это тяжело далось…», «Я делал то же самое».
Не на каждый вопрос нужно отвечать. Примите для себя сразу, что не на каждый ваш вопрос автор кода должен дать ответ. Так вы сможете задавать в своём отзыве вопросы, заставляющие задуматься. На них необязательно отвечать, чтобы влить код в кодовую базу, но зато они подстёгивают мысль разработчика и в долгосрочной перспективе повышают качество его работы.
Не нарушайте правило «Никаких сюрпризов»
Воспользуйтесь чек-листом:
Наверняка каждый участник вашей команды раз за разом совершает одни и те же десять ошибок. Причём труднее всего обнаружить пропуски каких-то элементов. Так что чек-лист — самый простой способ избежать наиболее распространённых ошибок и снизить вероятность пропусков. SmartBear, «Лучшие методики ревью кода»
Конечно, чек-лист не может покрыть все возможные случаи. Но в хороший список достаточно включить все требования, которым должен удовлетворять pull request, чтобы быть слитым с кодовой базой. Это полезный инструмент и для кодера, и для рецензента. Чек-лист может помочь улучшить код, сделать его более последовательным, избежать нарушения правил и стандартов. Новым участникам будет очень полезно законспектировать требования к коду, прежде чем отправлять свой первый pull request и проводить первую ревизию.
Пример чек-листа, с которого можно начать:
- В вашей ветке должна содержаться одна логически отдельная операция и никаких не связанных с ней изменений.
- У вас должны быть качественные commit-сообщения. См.
<ссылка на руководство по commit-сообщениям>. - Ваша ветка должна содержать новые или изменённые тесты для всего нового или изменённого кода. Все тесты должны выполняться в ветке. См.
<ссылка на руководство по написанию тестов>. - Ваша ветка должна содержать новую или обновлённую документацию для всего нового или обновленного кода. См.
<ссылка на руководство по написанию документации>. - Ваша ветка должна быть актуальна мастер-ветке и сливаться без конфликтов, поэтому сделайте ребейз до pull request«а.
- Весь новый код должен удовлетворять принятым у нас архитектурным требованиям и руководствам по Python, JavaScript, HTML и CSS-стилям. См.
<ссылка на руководства по архитектурам и стилям>. - Если новый код содержит изменения в схеме базы данных, то он должен включать и миграцию базы данных. См.
<ссылка на руководство по миграции базы данных>. - Если код содержит изменения, нарушающие обратную совместимость с плагинами, API-клиентами или темами, то насколько необходимы такие изменения? Эффект от таких изменений достаточно велик для отказа от совместимости? Внесён ли в список изменений отказ от обратной совместимости?
- Добавляет ли новый код какие-то зависимости (например, импортированы новые сторонние модули)? Если да, то проводилась ли оценка новых зависимостей, были ли они добавлены в соответствии с правильной процедурой? См.
<ссылка на руководство по обновлению зависимостей>. - Тестировался ли код с рабочими данными? См.
<ссылка на получение рабочих данных в руководстве разработчика>. - Если изменился пользовательский интерфейс, то проводилось ли тестирование на экранах разного размера и в разных браузерах? См.
<ссылка на руководство по адаптивному дизайну>. - Если изменился пользовательский интерфейс, то удовлетворяет ли он требованиям по доступности? См.
<ссылка на руководство по доступности>. - Если появились новые строковые переменные, видные пользователям, то были ли они интернационализированы? См.
<ссылка на руководство по интернационализации>.
Используйте исчерпывающе задокументированные стандарты программирования:
Стандарты программирования — это общее соглашение, заключаемое между разработчиками. Набор обязательных для всех рекомендаций по написанию качественного и удобного в сопровождении кода. Стандарты — фундамент ревью, потому что во время ревью мы проверяем код на соответствие стандартам. Вместо того чтобы предъявлять к коду требования, не входящие в стандарты, сделайте запрос на их включение в список рекомендаций.
Без доступного всей команде эталонного проекта стандартов разработчики могут даже не понимать, где обнаружатся проблемы при следующем ревью. А это не добавляет эффективности их работе.
Стандарты должны быть исчерпывающими. Можно опираться на стиль форматирования кода PEP 8, но это недостаточно полный стандарт для использования во время ревью.
Анализируйте то, что нужно, а остальным займутся инструменты
Во время ревью практически никогда не должны возникать проблемы с форматированием кода. Ревизии должны провоцировать появление продуктивных мыслей и дискуссий, а траты времени на стиль и форматирование в этом не помогут. Для подобных вещей используйте инструменты, например линтеры и средства автоматического форматирования.Заключение
Это статья о том, как давать положительные заключения и правильно критиковать, как успешно вносить предложения во время ревью кода. Я предлагаю вам сделать сжатую версию этого руководства с выделенными ключевыми моментами и раздать её каждому участнику команды. Положите инструкцию в общий репозиторий, чтобы любой мог начать её разбор и предложить свои изменения, и тогда вся команда будет вовлечена в обсуждение.
Также рекомендую подумать о том, как у вас создаётся код, который потом попадает на ревью. Вероятно, вы хотите, чтобы кодер и рецензент имели похожие представления о техническом дизайне до того, как код окажется на ревью. Заранее решите, кто будет писать код для конкретной фичи, а кто будет рецензировать, и поощряйте их работать вместе, чтобы эти двое обсуждали структуру кода и его промежуточные варианты, прежде чем проверять финальную версию. Нам же нужна качественная архитектура (две головы лучше), но также нужно избегать излишних дебатов во время анализа, когда оба разработчика предлагают совершенно разные решения (одному из них придётся полностью переписать уже готовый код).
Если ещё до ревью между автором и рецензентом установится крепкое сотрудничество, то во время ревью, скорее всего, всплывёт лишь несколько мелких проблем.
Комментарии (1)
5 января 2017 в 21:09
0↑
↓
Я считаю, что профессионалы должны быть готовы открыто обсуждать друг с другом результаты своей работы. В том числе во время обзора кода. Без грубости, спокойно, аргументированно, но открыто и честно.
Если код можно написать более эффективно и более идиоматично, то это нужно сделать. Если есть более эффективный алгоритм, то его нужно использовать. Либо аргументировать, почему в данной конкретной ситуации этого делать не нужно.
Мы технические специалисты, а не «художники с тонкой и хрупкой душевной организацией».
