Что такое ревью в айти
Еще одна статья о code review
Что такое code review
Что можно инспектировать
Для ревью подходит любой код. Однако, review обязательно должно проводиться для критических мест в приложении (например: механизмы аутентификации, авторизации, передачи и обработки важной информации — обработка денежных транзакций и пр.).
Также для review подходят и юнит тесты, так как юнит тесты — это тот же самый код, который подвержен ошибкам, его нужно инспектировать также тщательно как и весь остальной код, потому что, неправильный тест может стоить очень дорого.
Как проводить review
Вообще, ревью кода должен проводиться в совокупности с другими гибкими инженерными практиками: парное программирование, TDD, CI. В этом случае достигается максимальная эффективность ревью. Если используется гибкая методология разработки, то этап code review можно внести в Definition of Done фичи.
Из чего состоит review
Также очень важно определиться, за кем будет последнее слово в принятии финального решения в случае возникновения спора. Обычно, приоритет отдается тому кто будет реализовывать код (как в scrum при проведении planning poker), либо специальному человеку, который отвечает за этот код (как в google — code owner).
Как проводить design review
Design review можно проводить за столом, в кругу коллег, у маркерной доски, в корпоративной wiki. На design review тот, кто будет писать код, расскажет о выбранной стратегии (примерный алгоритм, требуемые инструменты, библиотеки) решения поставленной задачи. Вся прелесть этого этапа заключается в том, что ошибка проектирования будет стоить 1-2 часа времени (и будет устранена сразу на review).
Как проводить code review
Можно проводить code review разными способами — дистанционно, когда каждый разработчик сидит за своим рабочим местом, и совместно — сидя перед монитором одного из коллег, либо в специально выделенным для этого месте, например meeting room. В принципе существует много способов (можно даже распечатать исходный код и вносить изменения на бумаге).
Pre-commit review
Данный вид review проводится перед внесением изменений в VCS. Этот подход позволяет содержать в репозитории только проверенный код. В microsoft используется этот подход: всем участникам review рассылаются патчи с изменениями. После того как собран и обработан фидбэк, процесс повторяется до тех пор пока все ревьюверы не согласятся с изменениями.
Post-commit review
Данный вид review проводится после внесения изменений в VCS. При этом можно коммитить как в основную ветвь, так и во временную ветку (а в основную ветку вливать уже проверенные изменения).
Тематические review
Можно также проводить тематические code review — их можно использовать как переходный этап на пути к полноценному code review. Их можно проводить для критического участка кода, либо при поиске ошибок. Самое главное — это определить цель данного review, при этом цель должна быть обозримой и четкой:
Результаты review
Сложности при проведении review (субъективное мнение)
Основная сложность, с которой мы столкнулись, когда внедряли review в первый раз: это невозможность контроля изменений по результатам review. Отчасти это связано с тем, что данная практика применялась без других практик — CI (это еще раз доказывает тот факт, что все инженерные практики должны применяться вместе).
Утилиты для review
Вообще, утилит для проведения review существует большое количество, как платных, так и бесплатных. Я не стал их приводить, чтобы не навязывать свою точку зрения, в интернете можно найти множество инструментов и подробное описание.
Ссылки
Пожелания, дополнения, критика приветствуется
Что такое код-ревью и кто им занимается?
Эффективный способ заботиться о качестве кода
Проверка кода — это как базовое правило гигиены. Разработчики могут создавать запутанный и тяжелый код. Его сложнее обслуживать, а сбои появляются там, где не ждешь. Поэтому важная часть работы над продуктом — код-ревью, когда более опытные разработчики проверяют качество кода. Мы поговорили с Андреем Строговым, который руководит код-ревьюерами в Яндекс.Практикуме, о главных качествах такого профессионала и бесполезных комментариях к коду.
Задачи код-ревьюера
Код-ревью — это этап разработки кода. Чаще всего его проводят другие разработчики из той же команды. Так более опытные кодеры контролируют качество работы джуниоров или стажеров. Ревьюер на отдельных компонентах может показать, как сделать код проще и понятнее. Например, предложит взять функцию, которая уже написана для другого фрагмента. Проверка кода особенно важна для работы больших команд.
«В масштабных проектах код очень объемный и каждый разработчик знает только свой фрагмент. Люди часто не в курсе, что происходит в других компонентах и модулях. Это не слишком устойчивая ситуация, потому что автор кода может уйти в отпуск или по разным причинам перестать поддерживать свой фрагмент. Этап код-ревью добавляет второго человека, который понимает код и может с ним работать», — говорит руководитель команды код-ревью Андрей Строгов.
Код-ревью — это хороший способ договориться внутри команды, как писать код. Например, запутанный код сложно поддерживать в рабочем состоянии и масштабировать. Этап код-ревью помогает обмениваться знаниями, находить новые решения, делать лучше весь процесс разработки.
В отличие от тестирования, на код-ревью важнее разобраться в логике решения, чем найти ошибки. А еще — донести суть проблемы до разработчика. Для этого понадобится умение точно формулировать проблему и сообщать о ней без лишних эмоций.
«Когда мы проверяем код, не надо тратить время на мелкие ошибки — названия переменных, опечатки. Это плохо влияет и на того, кто пишет код, и на проверяющего. В первую очередь автору нужна обратная связь по логике кода. Проверку мелких ошибок легко автоматизировать», — говорит Андрей Строгов.
Этапы работы и инструменты
Код-ревьюер движется от общего к частному. Сначала ему нужно понять, какую задачу решал автор кода. Для этого проверяющий смотрит техническое задание и уточняет детали у разработчика. Дальше нужно оценить архитектуру кода и посмотреть, грамотно ли он написан. Это самый ценный этап код-ревью, он помогает избежать грубых ошибок и сэкономить время команде тестирования.
Когда ревьюер разобрался с задачей и логикой решения, он смотрит на функции, отдельные алгоритмы и их эффективность. Проверяет, можно ли заменить их другими методами и будет ли это лучше для всего продукта.
«На этих этапах не нужно никаких специальных инструментов. Только экспертные навыки и знание задачи. Код-ревьюеру понадобятся некоторые инструменты среды лишь для того, чтобы посмотреть, как работает код, и обнаружить грубые ошибки», — говорит Андрей Строгов.
После проверки ревьюер оставляет комментарии для разработчика. Его задача на этом этапе — объяснить, почему важно исправить ошибку. А еще проверяющий может подсказать решение или дать ссылки на материалы, с которыми разработчик быстрее приведет код в порядок. Весь процесс проверки не должен тормозить остальную разработку — хорошо, если код-ревью удается закончить за время рабочего дня.
«Наша задача в том, чтобы разработчик понял, в чём заключается комментарий и почему важно исправить код в соответствии с ним. Для этого недостаточно сильных технических знаний, нужны хорошие soft skills. Если ревьюер дал полезный комментарий, а разработчик почему-то не захотел исправлять — это будет выглядеть глупо», — говорит Андрей Строгов.
Полезный комментарий помогает исправить и улучшить код. А еще в нём нет субъективной оценки или нотаций. Поэтому критически важно, чтобы код-ревьюер умел давать качественную обратную связь. Иначе автор примет замечания на свой счет, и никакой эффективной работы не получится.
«Не стоит оставлять комментарии в духе “бред какой-то” или “тут ты не подумал”. Нужно формулировать проблему корректно. Субъективное мнение тоже не помогает улучшить код. Лучше найти что-то точнее, чем “это непонятный код”», — говорит Андрей Строгов.
Кроме комментариев код-ревьюер может оставлять рекомендации. Это некритичные замечания, их можно взять в работу, если у разработчика останется время. Например, проверяющий видит, что код выглядит запутанным. Задача решена, и весь функционал в порядке, но кажется, что работа сделана небрежно. Тут можно обойтись рекомендацией — обратить внимание человека на эту особенность его решения.
«Вы сэкономите время команде, если выделите критичные замечания. Это замечания, касающиеся фрагментов, которые могут привести к некорректной работе кода или помешают расширить его в будущем. Еще сюда относятся ошибки, из-за которых код трудно поддерживать и редактировать», — говорит Андрей Строгов.
Знания и навыки
Чтобы проверять код, нужно в нём разбираться. Хорошо, чтобы ревьюер уже решал такие задачи, писал подобный код и был знаком с тем стеком технологий, который используют в команде. Тогда ревьюер сможет дать разработчику полезные комментарии.
«Нужно отучить себя от того, что ты обязательно должен написать комментарии после ревью. Если с кодом всё в порядке, он может вернуться к автору без замечаний, которые оставляют ради самих замечаний», — говорит Андрей Сторогов.
Важно смотреть на код с позиции автора. У ревьюера может быть свой способ работы с кодом или другое решение для конкретной задачи. Но вся ценность его работы — предложить улучшения, ориентируясь на методы работы автора кода.
«Для команды хорошо, когда ревьюер может искренне похвалить удачное решение, — говорит Андрей Строгов. — Разработчики делятся на две категории. Одни считают, что пишут идеальный код, другие — что их код плох. Поэтому важно научиться искренне хвалить за хорошие решения. Это приятно автору кода и укрепляет отношения в команде».
Стать хорошим ревьюером помогает практика. Если в вашей команде пока нет такой деятельности, можно искать подходящие проекты на GitHub и оставлять комментарии там. «Код-ревью влияет на качество кода уже самим фактом своего существования, —говорит Андрей Строгов. — Когда знаешь, что твой код посмотрят, тщательнее к нему относишься. Например, постараешься его понятно оформить, не будешь использовать запутанную логику, в которой не смог бы разобраться другой разработчик. Это полезная социальная составляющая, которая мотивирует делать более понятный код».
Код ревью с учётом человеческих слабостей
Проверка кода (code review) — отличный инструмент для повышения качества кода, но он не учитывает один факт: отправляют и просматривают код люди, а они устают, теряют сосредоточенность, ленятся, да и просто испытывают эмоции в самые неожиданные моменты.
Поэтому хочу представить свое видение хороших и плохих практик код ревью с учётом человеческих особенностей.
Люди ленивы
Все люди ленивы. Под ленью я понимаю состояние экономии ресурсов, будь то физических или ментальных. Наши предки никогда не жили в изобилии ресурсов, поэтому организм научился их экономить, а лень — это один из эффективных инструментов.
Я заметил интересную особенность: если я пришлю на проверку небольшой кусок кода, то наверняка получу гораздо больше замечаний, чем если бы изменений было в разы больше. На первый взгляд, это нелогично, ведь чем больше кода, тем больше в нем проблем. Но если представить себе разработчика, который открывает страницу с изменениями и в ужасе видит 20 страниц нового кода, то становится понятно, почему он быстрее пытается его закрыть.
Открывая code review, разработчик рефлекторно оценивает количество работы, которую придется проделать, и с бо̒льшим рвением берется за маленькие изменения, поскольку на них потребуется меньше сил. Я считаю, что большие изменения почти никто не читает, разработчики просто стараются найти несколько дефектов, чтобы сделать видимость проверки.
Плохая практика: несколько спринтов пилить функциональность, а потом залить её одним большим pull request’ом.
Хорошая практика: делать изменения небольшими кусочками и создавать pull request’ы по мере выполнения задач. Даже если их будет смотреть один человек, ему будет проще при дозированности информации.
Перекладывание ответственности
В своей книге «5 пороков команды» Патрик Ленсиони явно показывает, что в команде начинаются проблемы, когда никто из сотрудников не хочет нести ответственность за результат.
Я достаточно часто встречаю ситуацию, когда весь код, который пишет команда, проходит через тимлида. В долгосрочной перспективе разработчики теряют ощущение, что они ответственны за создаваемую функциональность. Ответственность как бы делится между разработчиком и тимлидом. Разработчик начинает допускать мысль, что он может ошибиться, поскольку тимлид обязательно найдет проблему и исправит. Участники таких команд часто становятся безликими закрывателями тасок. А если тимлид уходит из компании, то команда оказывается недееспособной.
Вместо того, чтобы прогонять весь код через одного человека, стоит посмотреть в сторону практики перекрестной проверки, когда ревьюверы назначаются случайным образом. При таком подходе:
все знают, что происходит в проекте;
сотрудник понимает, что если он допустил ошибку, то виноват только он;
младшие разработчики могут учиться, читая код старших разработчиков;
тимлид не выгорает;
в случае ухода тимлида команда сможет продолжить доставлять функциональность.
Плохая практика: прогонять весь код через одного человека.
Хорошая практика: делать перекрестную проверку, когда все члены команды смотрят код друг друга.
Сложно меняют контекст
Наш мозг может обмануть сознание во многих ситуациях, и один из таких обманов — это многозадачность, точнее, иллюзия, что мы многозадачны. На самом деле это не так, мозг просто быстро меняет контекст. Но каждая смена контекста — это затраты энергии, а как мы уже выяснили, наш мозг ленив. В итоге проверяющему либо приходится тратить много сил на переключение контекста, либо он просто не погружается в код, ограничиваясь поверхностным просмотром.
Заканчивая работу над задачей, ты имеешь максимально полный контекст. Твоему коллеге же мало что понятно, и ему придется трать силы на восстановление этого контекста. К сожалению, без понимания проблемы, для решения которой была поставлена задача, невозможно оценить само решение. Поэтому для полноценного код ревью восполнять контекст всё-таки придется.
Помоги своему коллеге выстроить контекст. Для этого в code review есть описание. Туда надо поместить:
Причины, по которым эта задача вообще нужна. Чаще всего они описаны в тикете, поэтому можно добавить ссылку на него.
Как эта задача решалась и почему было выбрано именно это решение.
Всю документацию, которая получилась при работе над задачей.
Это поможет твоему коллеге восстановить контекст и глубже оценить написанное тобой решение.
Плохая практика: не делать описание, надеясь, что твой коллега в курсе, чем ты занимаешься.
Хорошая практика: делать подробное описание, которое включает в себя причины, по которым нужна эта задача, и описание решения с пояснением, почему задача решалась именно таким путем.
Люди не хотят глубоко погружаться в код
Если взять все замечания и отсортировать их по частоте появления, то получится вот такая последовательность:
Источник: YouTube канал Alex Four
Если посмотреть на эти замечания с точки зрения затрат сил на их обнаружение, то окажется, что проблемы, которые легко обнаружить, будут вверху списка, а проблемы, для поиска которых придется потрудиться, окажутся внизу. Это еще одно объяснение, почему люди не очень хотят глубоко погружаться в твой код.
Плохая практика: писать в code review про опечатки.
Хорошая практика: использовать линтеры и spell checker.
Еще одна особенность этого списка в том, что последние три типа проблем можно обнаружить только на код ревью и в продакшене. Особенно это касается проблем с безопасностью. Даже если ваша компания периодически проводит аудит безопасности, люди, которые будут его проводить, начнут, скорее всего, с просмотра кода, и тем самым сделают своеобразное code review.
Отличной практикой будет ведение чек-листов со списком того, на что нужно смотреть во время проверки кода. Например:
поиск потенциальных проблем с безопасностью;
проблемы с производительностью;
оценка выбранного архитектурного решения;
Плохая практика: не искать в новом коде проблемы из конца списка.
Хорошая практика: использовать чек-листы и проверять по ним возможные проблемы.
Люди эмоциональны
Если бы по обе стороны код ревью сидели «эконы», однако в этом процессе участвуют обычные люди, которые склонны испытывать эмоции, зачастую заставляющие нас вести себя не очень разумно.
Принимаем замечания на свой счет
Самый яркий пример нерационального поведения — принимать замечания на свой счет. Эконы понимают, что замечания должны улучшить код проекта и в долгосрочной перспективе принести пользу всем.
К сожалению, на практике это работает иначе. Любой человек в долгосрочной перспективе пытается оградить себя от деятельности, которая связана с негативом, и разработчики не исключение. Программист, который постоянно получает негативную обратную связь от коллег, становится безынициативным, теряет интерес к написанию кода и, как следствие, перестает развиваться.
Не связывать код с человеком, его написавшим
При написании комментария к ошибке необходимо избегать местоимения «ты». Для примера, вот безобидный комментарий: «Ты допустил ошибку вот в этом выражении. ». Но поскольку общение происходит в виде переписки, то читающий этот комментарий в зависимости от своего настроения, отношений с коллегами, уверенности в себе и еще кучи факторов может интерпретировать эту фразу двумя способами:
Эй, приятель! Ты ошибся в этом выражении. Но я все равно считаю, что ты умный! Вероятно, это просто опечатка.
Ты допустил ошибку в этом выражении, ты вообще знаком с синтаксисом языка? Кто вообще так пишет?
К счастью использования «ты» можно легко избежать, просто убрав объект из предложения: «Кажется, в этом выражении ошибка». Либо вообще составить предложение в виде вопроса: «Может, попробовать описать это при помощи конструкции switch/case?»
Оформление комментария в виде вопроса — хорошая практика, поскольку это оставляет человеку выбор; тогда как комментарий, оформленный в виде приказа (например, «Используй в этом месте switch/case»), заставляет разработчика занимать оборонительную позицию.
Плохая практика: использовать местоимение «ты» в комментарии к ошибке и писать комментарии в повелительном наклонении.
Хорошая практика: использовать обезличенные предложения. Составлять комментарии в виде вопроса, давая читающему возможность согласиться или поспорить с утверждением.
Программисту, отправляющему код на проверку, стоит отделять отношение к себе от кода. Если кто-то ругает твой код, то не стоит считать, что он ругает тебя. Попробуйте отнестись к этому как к возможной точке для профессионального развития.
Плохая практика: считать, что человек, критикующий твой код, критикует тебя.
Хорошая практика: воспринимать критику как одну из точек для профессионального роста.
Конфликтность vs важность
Если замечания, которые возникают при проверке кода, разделить на четыре группы в зависимости от конфликтности и важности, то получится вот такая схема:
Источник: YouTube канал Alex Four
В идеале, ошибки нужно заводить только по замечаниям из четвертой группы, поскольку они наиболее конструктивны, полезны для проекта и вызывают наименьший негатив.
Проблемы из третьей группы должны исправляться автоматически при помощи линтеров и spell checker`ов.
Проблемы из первой группы я бы вообще отнес к вкусовщине: это то, что мало влияет на работу проекта в целом, зато очень сильно раздражает.
Если в вашей команде при проверке часто возникают замечания из первой группы, то стоит ввести роль модератора — это человек, который не участвует в проверке, но следит за адекватностью дискуссий. Если обсуждение постепенно переходит в плоскость холивара, модератор должен остановить это.
Плохая практика: холиварить во время код ревью.
Хорошая практика: приглашать модератора дискуссий в код ревью.
Если у вас в команде часто возникают замечания из второй группы, которые, с одной стороны, важны для качества кода проекта, а с другой — могут стать причиной конфликтов, то стоит ограничить возможности текстовой переписки.
Если проверяющий заводит ошибку, то у написавшего код есть всего две возможности:
Молча согласиться и исправить.
Встретиться и лично обсудить вопрос, а после встречи занести решение в code review.
Дело в том, что во время личного обсуждения вероятность неправильной интерпретации фразы меньше.
Возможно, в начале встреч станет больше и вам будет о чем подискутировать с коллегами, но в итоге количество встреч придет в норму, а внутри команды выработается общее представление о подобных ситуациях.
Такие встречи не должны быть длинными, договориться можно за 15-20 минут. После 30-40 минут конструктивное обсуждение заканчивается, а диалог перетекает в холивар. Если подобное происходит регулярно, то не стесняйтесь приглашать на такие встречи модератора, возможно даже из других команд или направлений.
Плохая практика: вести текстовые дискуссии.
Хорошая практика: обсуждать замечания лично.
Вывод
Код ревью может подсвечивать проблемы в команде. Это могут быть конфликты в процессах или межличностных отношениях. Например, отправка большого количества кода говорит о проблемах с декомпозицией, а постоянные неконструктивные замечания от одного из разработчиков могут говорить о личных конфликтах внутри коллектива. Если помнить, что проверкой занимаются люди со своими особенностями и эмоциями, то она станет отличным инструментом для поддержания кодовой базы в чистоте.
Перфоманс ревью и обратная связь в IT
Некоторые компании до сих пор считают, что перфоманс ревью — бесполезная трата времени, особенно в IT компаниях. Хотя обратная связь, которую можно наладить с его помощью, частично закрывает боль «И снова он сбежал». Если мы посмотрим на мир за пределами нашей страны, то увидим, что довольно большой процент компаний использует мотивацию и ревью, как способ сделать компанию лучше. Среди таких можно перечислить: Google с ее системой OKR, Adobe с ее постоянной обратной связью, General Electric с собственным приложением для повышения производительности и другие компании.
Так что же это за метод такой?
По факту, ревью представляет собой череду действий, позволяющих выявить «слабые» и «сильные» места в компании и подтянуть «слабые». Если проводить параллели, то это часть фреймворка, которая показывает, где у вас что «тупит», а вся задача его — исправить ошибки. Важным винтиком процесса являются люди, которые будут оценивать других людей, разбирать их проекты, оценивать и давать конкретные рекомендации и даже цели. Поэтому не стоит надеяться только на HR отдел или аутсорс. В этом процессе обязательно должны участвовать менеджеры отделов, причем активно.
Кому он нужен?
Метод показан компаниям, которые насчитывают несколько десятков человек и более. Именно при таких размерах начинают происходить заметные потери при взаимодействии между командами и отделами. В более мелких компаниях можно обойтись и без ревью, если в обозримом будущем не планируется рост и развитие.
Менеджеры должны понимать свою ключевую роль в этом процессе. Они являются теми, кто определяет стратегию и ценности компании, кто должен следить за правильным их принятием и пониманием остальной частью команды.
Характеристики хорошего перфоманс ревью
Как выстраивается процесс?
Несколько советов напоследок
СОВЕТ № 1: помните, что конструктивная обратная связь используется, чтобы помочь сотруднику определить, что пошло не так, и как он может лучше справиться с ситуацией в будущем. Изложите свои выводы и предложенный процесс в письменной форме, чтобы у сотрудника были четкая инструкция по предлагаемому решению. Таким образом, не возникнет недопонимания.
СОВЕТ № 2: важно адаптировать свое общение к стилю работы сотрудника и его уникальным мотивам. Сотрудник, который непреклонен в карьерном росте, может лучше отреагировать на конструктивную обратную связь. Если объяснить, как его действия повлияют на его будущую роль в компании. Знание того, что движет сотрудником, поможет вам связать конструктивную обратную связь с их реальными амбициями в вашей компании, что еще больше повысит их вовлеченность и производительность.
СОВЕТ № 3. Если сотрудник плохо работает, задавайте вопросы. Не думайте, что вы знаете причину, и не делайте поспешных выводов о том, что он ленив, она тупица, он немотивирован или она некомпетентна. Используйте свои разговоры об оценке эффективности как возможность выяснить, каковы возможные причины неспособности сотрудника соответствовать стандартам и ожиданиям. Подсказка: когда сотрудник не работает должным образом, основной причиной часто является неспособность начальника обучать!
Попробуйте разработать простую, удобную в использовании систему, которая побуждает начальников и сотрудников участвовать в двустороннем общении в течение года — это единственный способ получить реальную пользу от системы проверки эффективности.
Вы можете связаться с нами для получения гайда по разработке перфоманс-ревью и мы с радостью его вышлем.