Code review по-человечески (часть 1)
В последнее время я читал статьи о лучших практиках code review и заметил, что эти статьи фокусируются на поиске багов, практически игнорируя другие компоненты ревью. Конструктивное и профессиональное обсуждение обнаруженных проблем? Неважно! Просто найди все баги, а дальше само сложится.
Так что у меня случилось откровение: если это работает для кода, то почему не будет работать в романтичных отношениях? Итак, встречайте новую электронную книгу, которая поможет программистам в отношениях со своими возлюбленными (обложка на иллюстрации слева).
Моя революционная книга обучит вас проверенным техникам по выявлению максимального количества недостатков в своём партнёре. Книга не затрагивает следующие области:
• Обсуждение проблем с сочувствием и пониманием.
• Помощь партнёру в устранении недостатков.
Насколько я могу понять из чтения литературы по code review, эти части отношений настолько очевидны, что вообще не стоят обсуждения.
Как вам нравится такая книжка? Предполагаю, что она вам не очень по душе.
Так почему мы проводим code review таким образом?
Могу лишь предположить, что прочитанные мною статьи — из будущего, где все разработчики являются роботами. В этом мире ваши коллеги приветствуют беспечную критику своего кода, потому что обработка такой информации согревает их холодные металлические сердца.
Я собираюсь сделать смелое предположение, что вам хотелось бы улучшить code review в настоящем, где вы работаете с людьми, а не с роботами. Даже более смелое предположение, что хорошие отношения с коллегами — это главная цель, а не просто переменная для ускорения исправления ошибок (минимизации параметра cost-per-defect). Как бы изменились ваши методы ревью с учётом этих обстоятельств?
В этой статье обсудим техники, которые предполагают, что code review — не только технический, но и социальный процесс.
Что такое code review?
Термин “code review” может означать разные действия, от простого чтения какого-то кода из-за спины разработчика до совещания на 20 человек, где вы разбираете код строчка за строчкой. Я здесь имею в виду формальную и письменную процедуру, но не отягощённую рядом совещаний для инспекции кода.
В код ревью принимают участие автор, который написал код и отправил его на ревью, и рецензент, который анализирует код и принимает решение, когда тот готов для добавления в общую кодовую базу проекта. В процессе может участвовать несколько рецензентов, но для простоты предположим, что он один.
Перед началом ревью автор должен создать список изменений, где перечисляет все изменения, которые хочет внести в общую кодовую базу.
Ревью начинается, когда автор отправляет список изменений рецензенту. Оно происходит в несколько раундов. Каждый раунд — это один полный цикл приёма-передачи между автором и рецензентом: автор присылает изменения, рецензент отвечает отзывом на эти изменения. Каждая рецензия кода состоит из одного или нескольких раундов.
Ревью завершается, когда рецензент одобряет изменения. Обычно этому сопутствует отправка сообщения LGTM, сокращённой фразы “looks good to me”.
Почему это так трудно?
Если программист присылает вам список изменений, которые он считает потрясающими, а вы отвечаете ему подробным перечислением причин, почему он не прав, такое общение требует определённой деликатности.
«Это одна из причин, почему я не скучаю по IT, ведь программисты — весьма малопривлекательные люди… Например, в авиации те, кто слишком переоценивают свои способности, уже мертвы».
Филип Гринспан, сооснователь ArsDigita, из книги «Основатели за работой».
Автору очень легко воспринять критику своего кода в том смысле, что он некомпетентный программист. Рецензия кода — это возможность обмениваться знаниями и принимать осмысленные инженерные решения. Но это невозможно, если автор воспринимает обсуждение как персональную атаку на него самого.
Как будто и без этого недостаточно сложностей, здесь вами приходится выражать свои мысли в письменной форме, что ещё больше повышает риск недопонимания. Автор не слышит тон вашего голоса, не может воспринять язык тела, поэтому так важно аккуратно подбирать формулировки. Для автора, который занял оборонительную позицию, безобидное примечание вроде «Ты забыл закрыть файловый дескриптор» может звучать как «Не могу поверить, что ты забыл здесь закрыть файловый дискриптор! Ты такой идиот!»
Техники
Отдайте компьютерам скучную часть работы
Когда вас постоянно отвлекают совещания и почтовые письма, трудно выкроить время для вдумчивого анализа кода. Ментальных сил ещё меньше, чем времени. Чтение кода коллеги — когнитивно тяжёлая задача и требует высокого уровня концентрации. Не транжирьте эти ресурсы на задачи, которые может выполнить компьютер, особенно если он выполняет их лучше.
Очевидный пример — ошибки с пробелами в отступах. Сравните, сколько усилий требует выявление такой ошибки от человека, по сравнению с применением автоматического инструмента форматирования:
Правая часть пустая, потому что автор использовал редактор кода, который автоматически форматирует пробелы каждый раз при нажатии кнопки «Сохранить». Ну худой конец, когда автор отправляет свой код на проверку, система непрерывной интеграции сообщает о неправильных пробелах. Автор исправляет проблему ещё до того, как рецензент её заметил.
Посмотрите на другие механические действия, которые можно автоматизировать, вот наиболее часто встречающиеся:
| Задача | Автоматизированное решение |
|---|---|
| Проверить билды | Система непрерывной интеграции, такая как Travis или CircleCI. |
| Проверить прохождение автоматических тестов | Система непрерывной интеграции, такая как Travis или CircleCI. |
| Проверить, что отступы и пробелы соответствуют корпоративному стилю. | Инструмент форматирования кода, такой как ClangFormat (для C/C++) или gofmt (для Go). |
| Идентификация неиспользуемых модулей или неиспользуемых переменных | Статические анализаторы кода, такие как pyflakes (линтер для Python) или JSLint (линтер для JavaScript). |
Автоматизация помогает вам как рецензенту внести более ценный вклад. Если игнорировать целый класс проблем, вроде порядка импорта модулей или соглашения о присвоении имён для файлов исходников, то вы можете сконцентрироваться на более интересных вещах, таких как функциональные ошибки или недостаточная читаемость кода.
Автоматизация выгодна и автору. С её помощью он может обнаружить небрежные ошибки за несколько секунд, а не за несколько часов. Мгновенный фидбек ускоряет обучение и упрощает исправление ошибки, потому что релевантный контекст у автора ещё в рабочей памяти. Кроме того, автор гораздо легче воспринимает сообщение о глупой ошибке от компьютера, а не от вас.
Нужно всем вместе поработать, чтобы внедрить автоматические проверки такого рода в рабочий процесс code review (например, хуки перед коммитами в Git или вебхуки в GitHub). Если процесс ревью требует от автора запускать такие проверки вручную, то вы лишаетесь большей части преимуществ. Автор неизбежно иногда будет забывать о проверке, из-за чего вам придётся возиться с с простыми ошибками, которые могла исправить автоматическая проверка.
Оформите аргументы по стилю в виде руководства по стилю
Обсуждение стиля — это потерянное время в процессе код ревью. Разумеется, единообразный стиль важен, но обсуждение кода — неподходящее время для прений, где поставить фигурные скобки. Лучший способ исключить споры и стиле из всего процесса — завести руководство по стилю.
Хорошее руководство по стилю определяет не только внешние элементы оформления вроде соглашения о присвоении имён или правила отступов, но и как использовать функции данного языка программирования. Например, JavaScript и Perl набиты функциональностью — там есть много вариантов реализации одной и той же логики. Руководство по стилю определяет Один Правильный Способ программирования, так что вы больше не увидите, что одна половина вашей команды использует один набор функций языка, а другая половина — совершенно другой набор функций.
Как только у вас появилось руководство по стилю, больше не придётся тратить циклы ревью на обмен сообщениями с автором в спорах, каким способом лучше именовать файлы. Просто следуйте руководству и двигайтесь дальше. Если в вашем руководстве отсутствует инструкция по определённому вопросу, обычно и не стоит о нём спорить. Если вы встречаетесь с вопросом по стилю, который не описан в руководстве, но достоин обсуждения, решите его вместе с командой. Затем запишите решение в руководство по стилю, чтобы больше никогда не возвращаться к этому обсуждению.
Вариант 1: адаптация существующего руководства по стилю
Если поискать в интернете, то можно найти опубликованные руководства по стилю — и позаимствовать их для собственного использования. Самые известные — руководства по стилю от Google, но можно найти и другие, если эти не подходят. Адаптируя существующий документ, вы получаете все выгоды от наличия руководства по стилю, не тратя времени на написание такого документа с нуля.
Недостаток в том, что все организации адаптируют эти правила для внутренних нужд. Например, руководства Google очень консервативны относительно использования новых функций языков, потому что у них огромная кодовая база, которая должна работать на всём: от домашнего маршрутизатора до последнего iPhone. Если у вас стартап из четырёх человек с единственным продуктом, то логично выбрать более агрессивную стратегию в использовании самых последних функций или расширений языков.
Вариант 2: Постепенно дополняйте собственное руководство по стилям
Если не хотите адаптировать существующий документ, можно создать свой собственный. Каждый раз, когда во время код ревью возникает дискуссия по стилю, поднимайте перед всей командой вопрос, каким должно быть официальное соглашение. Когда достигнете согласия, закрепляйте это решение в руководстве по стилям.
Я предпочитаю держать наше руководство в формате Markdown в системе контроля версия (например, на GitHub Pages). Так все изменения проходят через стандартную процедуру рецензирования — кто-то должен явно одобрить изменения, а каждый в команде может поднять любую проблему. Вики и Google Docs тоже подходят.
Вариант 3: Гибридный подход
Сочетая варианты 1 и 2, вы можете адаптировать существующее руководство и одновременно вести локальный документ для расширения или перезаписи базы. Хороший пример — руководство Chromium C++. Там в качестве базы взяли руководство по C++ от Google, но сделали собственные изменения и дополнения.
Начинайте ревью немедленно
Расценивайте code reviews как задачу с высоким приоритетом. Когда вы изучаете код или пишете отзыв, не торопитесь, но начинайте делать это немедленно — в идеале, в течение нескольких минут.
Если коллега прислал список изменений, вполне вероятно, что его работа приостановилась до получения вашего отзыва. В теории, система контроля версий позволяет создать ещё одну ветку и продолжить работу, а затем смержить изменения в ревью с новой веткой. В реальности, в мире есть примерно четыре разработчика, которые эффективно умеют такое делать. У всех остальных распутывание трёхсторонних дифов занимает столько времени, что нивелирует любой прогресс, которого можно добиться в ожидании ревью.
Если вы начинаете ревью немедленно, то создаёте благоприятный цикл. Весь документооборот становится исключительно функцией размера и сложности списка изменений автора. Это побуждает авторов присылать небольшие, точно сформулированные списки. Их легче и приятнее рассматривать, так что ваши ревью ускорятся, а благоприятный цикл продолжится.
Представьте, что ваш коллега реализовал новую функцию, которая требует изменения 1000 строчек кода. Если он знает, что вы можете сделать ревью списка изменений на 200 строчек примерно за два часа, то разобьёт эту функцию на фрагменты примерно по 200 строчек и закончит ревью всей функции за один-два дня. Но если любые код ревью занимают у вас целый день, независимо от размера, то утверждение функции займёт неделю. Ваш коллега не хочет ждать целую неделю, так что он скорее направит вам более крупные списки, по 500-600 строк каждый. Их труднее рассматрвиать, да и отзывы станут беднее, потому что тяжелее сохранить контекст для изменения на 600 строк, чем на 200.
Абсолютным максимумом продолжительности одного раунда ревью должен быть один рабочий день. Если вы заняты проблемой с ещё большим приоритетом и не укладываетесь в один рабочий день, сообщите коллеге об этом и дайте ему возможность выбрать для ревью кого-то другого. Если вам приходится отказываться от ревью чаще, чем раз в месяц, то вероятно, что команде нужно снизить темп гонки, чтобы сохранить вменяемые практики разработки.
Начните с высокого уровня, и спускайтесь ниже
Чем больше заметок вы пишете в каждом конкретном раунде, тем больше риск, что автор будет чувствовать себя подавленным. Точный лимит зависит от разработчика, но опасная зона обычно начинается в районе 20-50 заметок на один раунд ревью.
Если не хотите утопить автора в море замечаний, ограничьте себя только высокоуровневыми правками в первом раунде. Сконцентрируйтесь на проблемах вроде перепроектирования интерфейса класса или разбиения сложных функций. Подождите исправления этих проблем, прежде чем переходить к низкоуровневым вопросам, таким как именование переменных или понятность комментариев в коде.
Ваши низкоуровневые заметки могут стать неактуальными, когда автор сделает высокоуровневую правку. Перенеся их на более поздний раунд, вы оградите себя от нетривиальной работы по тщательному подбору слов для комментариев на эти спорные темы, а автору не придётся обрабатывать необязательные замечания. Такая техника также сегментирует уровни абстракции, на которых вы концентрируетесь в процессе ревью, помогая вам и автору ясно и систематически пройтись по списку изменений.
Щедро используйте примеры кода
В идеальном мире автор кода будет благодарен за любое ревью. Для него это возможность учиться и защита от ошибок. В реальности много внешних факторов, из-за которых авторы могут негативно воспринять ревью и возмущаться, что вы делаете замечания к их коду. Может, они спешат из-за дедлайна, так что любая заминка, кроме мгновенного одобрения, кажется препятствием. Может, вы не так долго работаете вместе и они не верят, что ваше замечание сделано из благих намерений.
Хороший способ улучшить восприятие код ревью автором — найти возможность подарить ему подарок. А какие подарки любят получать все разработчики? Конечно, примеры кода.
Если вы облегчите работу автора тем, что сами напишете некоторые изменения, которые предлагаете сделать, то продемонстрируете, что вы щедро делитесь своим временем как рецензент.
Предположим, ваш автор не очень хорошо знаком с функцией списковых включений в Python. Он присылает на рецензию код с такими строчками:
Ответ в стиле «Можно это упростить с помощью списковых включений?» вызовет раздражение, потому что ему нужно потратить 20 минут, изучая функцию, которую никогда раньше не использовал.
Автор будет гораздо счастливее получить такую заметку:
Предлагаю рассмотреть списковое включение такого рода:
Эта техника не ограничивается однострочными изменениями. Я часто создаю собственную ветку кода для демонстрации автору большой концептуальной идеи, такой как разбиение крупной функции или добавление юнит-теста для покрытия дополнительной граничной ситуации.
Применяйте эту технику только для ясных, бесспорных улучшений. В вышеприведённом примере спискового включения немногие разработчики будут спорить с сокращением количества строк кода на 83%. И наоборот, если вы напишете пространный пример для демонстрации изменения, которое «лучше» на ваш личный вкус (например, изменения стиля), то такой пример кода создаст впечатление, что вы настырно проталкиваете свои предпочтения, а не проявляете щедрость.
Ограничтесь двумя-тремя примерами кода на каждый раунд. Если начнёте писать код для всего авторского списка изменений, то это создаёт впечатление, как будто вы не считаете самого автора способным написать этот код.
Никогда не говорите «ты»
Это звучит немного странно, но послушайте: никогда не обращайтесь лично к автору в процессе код ревью.
Ваши решения должны быть основаны на идеях по улучшению кода, а не на том, кто придумал эти идеи. Ваш коллега потратил немало сил на свой список изменений и код и, наверное, гордится проделанной работой. Естественная реакция на критику его работы — стать в оборонительную позу.
Подбирайте слова для отзыва таким образом, чтобы минимизировать риск возникновения оборонительной позы. Чётко обозначьте, что вы критикуете код, а не самого разработчика. Когда автор видит в комментарии личное обращение к нему, это отвлекает его внимание от кода и переводит внимание на личность. Так повышается риск, что он воспримет критику лично.
Например, вот безобидный комментарий:
Ты допустил ошибку в слове «благополучно».
Автор может интерпретировать такое замечание двумя соврешенно разными способами:
Вторая заметка — это просто исправление, а не оценка автора.
К счастью, можно легко переписать свои комментарии, избегая слова «ты».
Вариант 1: Замените «ты» на «мы»
«Мы» усиливает коллективную ответственность всей команды за код. Автор может перейти в другую компанию, как и вы, но в том или ином виде останется команда, которая отвечает за этот код. Может показаться глупым говорить «мы», когда речь идёт об изменении, которое автор явно должен сделать единолично, но лучше выглядеть глупым, чем обвинителем.
Вариант 2: Удалите из предложения субъект
Другой вариант избежать личного обращения — использовать сокращение, которое исключает из предложения субъект:
Того же эффекта можно достичь с помощью пассивного залога. Я обычно в технических текстах избегаю пассивного залога как чумы, но это может быть полезный вариант обхода проблемы с личным обращением:
Ещё один вариант — перефразировать предложение в виде вопроса, который начинается со слов «Что насчёт. » или «Как насчёт. »:
Оформляйте отзывы как запросы, а не команды
Код ревью требует большего такта и осторожности, чем обычное общение, потому что здесь выше риск, что обсуждение скатится в личный спор. Казалось бы, рецензенты должны проявлять бóльшую вежливость и учтивость в код ревью по сравнению с личным общением. Но я обнаружил страннейшим образом прямо противоположную ситуацию. Многие люди никогда не скажут коллеге, «Дай мне этот степлер, а потом принеси газировки». Но я видел множество случаев, когда рецензенты оформляют отзывы в таком командном стиле, вроде «Перенеси этот класс в отдельный файл».
Но и не будьте раздражающе вежливым в своих комментариях. Оформляйте их как запросы или предложения, а не как команды.
Сравните одно и то же замечание, оформленное двумя разными способами:
| Замечание, оформленное как команда | Замечание, оформленное как запрос |
|---|---|
| Перенеси класс Foo в отдельный файл. | Мы можем перенести класс Foo в отдельный файл? |
Людям нравится контролировать свою работу. Такой запрос даёт автору чувство самостоятельности.
Такие запросы также повышают шансы, что автор ответит вежливо. Может быть, у него есть свои причины сделать такой выбор. Если вы оформляете замечание в виде команды, то любое несогласие автора принимает форму неповиновения. Если оформить замечание как запрос или вопрос, то автор может просто ответить.
Сравните, насколько воинственной кажется беседа в зависимости от оформления первоначального замечания:
Видите, насколько более цивилизованным стало общение, когда вы сконструировали воображаемый диалог в доказательство своего тезиса оформили замечание в виде запроса, а не команды?
Обосновывайте принципами, а не мнениями
Когда составляете замечание для автора, то объясните и предлагаемое изменение, и его причину. Вместо фразы «Нам следует разделить этот класс на два» лучше сказать «Сейчас этот класс отвечает одновременно и за скачивание файла, и за его парсинг. Следует разделить его на класс для скачивания и на класс для парсинга согласно принципу единой ответственности».
Если обосновывать замечания принципами, то дискуссия принимает конструктивную форму. Когда вы цитируете конкретную причину, вроде «Нужно сделать эту функцию приватной, чтобы минимизировать открытый интерфейс класса», то автор не может просто ответить «Нет, я предпочитаю сделать по-своему». А если он так ответит, то это будет выглядеть глупо, потому что вы показали, как изменение позволяет достичь цели, а он просто заявил о своём предпочтении какому-то способу.
Разработка программного обеспечения — это одновременно искусство и наука. Не всегда можно точно сформулировать, что именно неправильно в данном фрагменте кода с точки зрения устоявшихся принципов. Иногда код просто некрасивый или излишне усложнённый, и сложно точно сформулировать, почему. В таких случаях объясните как можете, но сохраняйте объективность. Если сказать «Мне кажется, это сложно понять», то это хотя бы объективное утверждение, в сравнении с фразой «Это запутанный код», что является оценочным суждением, с которым не каждый может согласиться.
Предоставляйте подтверждающие доказательства в форме ссылок, где это возможно. Соответствующая секция руководства по стилям вашей команды — наилучшая ссылка, какую вы можете предоставить. Вы можете также сослаться на документацию для языка или библиотеки. Ответы StackOverflow с высокими рейтингами тоже подходят, но чем дальше вы отходите от авторитетной документации, тем более зыбкими становятся доказательства.
Часть 2: скоро
Через пару недель я опубликую вторую часть статьи. Будьте на связи, там рассмотрим дополнительные советы, в том числе:
Как завести у себя в команде код-ревью. Отвечаем на вопросы
В рамках предыдущей статьи «По следам код-ревью» меня попросили написать поподробнее про инспекцию кода в своей команде:
Интереснее как такое внедрить в команде:
— кто проводит
— с какой периодичностью
— как исправляете (задачу создаете или еще как)
— как контролируете выполнение исправления (вдруг разработчик просто забил на замечание)
— как боретесь с вкусовщиной (когда у разных разработчиков разное понимание стандартов)
— всех проверяете или выборочно
Но про организационные аспекты было бы не менее интересно узнать.
На каких проектах код-ревью целесообразно применять?
Какие трудности возникают?
Как долго применяете?
Сколько времени тратится и каких специалистов?
Очень полезно было бы узнать ответы на эти вопросы.
У меня появилось немного свободного времени, так как троих мелких я развез по бабушкам на текущий месяц, то выполняю свое обещание. Рекомендации базирую на своем опыте, выражаю свое мнение и результаты размышлений. Саму статью я решил сделать в формате вопрос и ответ.
Во-первых, хочу обратить внимание, что для код-ревью вы и ваша команда должны «созреть»! Учтите, убеждать вас внедрять эту методологию с пеной у рта, что это должно быть везде и у всех я не буду.
Во-вторых, этот процесс внедрения является самым настоящим проектом, поэтому у него должен быть руководитель – тот человек, который начнет и завершит его. Само собой, это не поедет.
В-третьих, у каждой команды свой особенный и индивидуальный процесс, поэтому процесс запуска и результат на выходе будут иметь какие-то свои особенности. Возможно какие-то рекомендации или шаги придется дорабатывать под себя.
Если по первым пунктам вы готовы, тогда поехали!
Что такое код-ревью?
Что нам дает код-ревью?
Разработка без code-review и с code-review.
Нам обзор кода привнес следующие бонусы (простыми словами):
Что требуется для внедрения методики код ревью в процесс разработки?
Как внедрить код-ревью?
По части тезисов будут расшифровки ниже по тексту.
Дорожная карта от желания «хочу внедрить код-ревью» до успешного использования в команде
Кто может выполнять код-ревью?
Для выполнения код-ревью должны быть выбраны наиболее опытные специалисты универсалы или более узкие специалисты в некоторых направлениях. И далее назначения ответственными на задачи в части код-ревью выбираются в зависимости от сложности задачи и ее области изменения.
Далее по достижении некоторого профессионального уровня к процессу подключаются бывшие джуны.
Также для ответственного по код-ревью у нас есть маленькая инструкция шпаргалка, на что обязательно смотреть и что проверять.
Существует мнение код-ревью должен выполнять каждый участник команды выбираемы рандом, но. Вот тут крайне не согласен, вы бы доверили провести код-ревью джуну, когда на кону стоит изменение механизма взаиморасчетов или отправки данных в налоговую?
Либо код-ревью должна проводить вся команда. Не согласен также, конечно здорово если все 10 человек посмотрят каждую задачку, но когда кодить будем? Команда привлекается (или ее большая часть) в моменты кода возникает спор между участниками (эскалация), либо они сами призывают для помощи других участников.
Задачи на код-ревью выдаются на команду и может взять кто-то любой? Это здорово, но всегда будет какая-то сложная задача или сложный автор, которого будут всячески избегать, и еще если задачка не очень важная. Да, еще все будут думать друг на друга: Вася сделает, а Вася будет думать, что пусть Петя возьмет. При таком подходе у нас на ревью задача как-то висела месяц, она была еще и не особо важная (низкий приоритет).
Должен быть назначен один ответственный по код-ревью? А вот дальше можно меняться задачками друг с другом, совещаться, призывать коллективный разум.
Нужен ли кто-то для контроля?
Обязательно должен быть ответственный человек, который все этим управляет и контролирует (по крайней мере на старте). Если не будет ответственного, то это все дело обязательно скатится в хаос и нечего не получится.
Как выглядит процесс разработки с учетом инспекции кода?
Привожу пример процесса с использованием инспекции кода:
Задача принимается в работу, а после отправляется в соответствующий статус, который требует проведения обзора кода (можно провести аналогию с pull-request).
Далее ответственное лицо принимает задачу в работу (статус «Код-ревью») и проверяет. Результатом его работы будет принятие задачи или возврат на доработку (более подробно процесс взаимодействия с примерами см. «Процесс проведения код-ревью?»).
На рисунке приведена доска SCRUM (см. ниже «Пример доски задач SCRUM»), которая содержит в себе представление процесса разработки.
Пример доски задач SCRUM
Процесс проведения код-ревью?
Мы проводим код-ревью в рамках бизнес процесса. Если задача не прошла код ревью, тогда она возвращается в работу. И после завершения опять переводится на код-ревью.
Наличие подтвержденного код-ревью у нас проверяется при формировании сборки.
Результатом код-ревью в задаче должна быть отдельная отметка [КОД-РЕВЬЮ] с информацией о том, что она пройдена.
В случае наличия замечаний, то автору приводится краткий список проблем:
[КОД-РЕВЬЮ]
1. Изменить форматирование
2. Используем запросную модель, а не объектную в модуле объекта….
3. Возможна ошибка при использовании…
Автор при возврате отвечает оппоненту:
Как погасить эскалацию конфликта ревьювера и автора?
В процессе выполнения диалога между автором и проверяющим может произойти тупиковая ситуация. Когда автор не согласен с замечаниями инспектора, а инспектор по коду не пропускает решение оппонента. Тогда должна происходить эскалация обсуждения на более высокий уровень и могут привлекаться коллеги или призываться сразу «арбитр».
Последней инстанцией при эскалации споров должен являться самый опытный сотрудник, архитектор (главный судья или арбитр) или что-то вроде комиссии. Оставлять последнее слово за автором не совсем хорошая идея.
Что такое дизайн-ревью (desing-review)?
Благодаря использованию desing-review вы многократно усиливаете эффект code-review. Тем самым вы устраняете ошибку выбора проектного решения, до момента когда ее устранить довольно сложно или уже не возможно.
Для разработчика вы разбиваете задачу на два этапа:
Когда вы выполняете этот пункт то необходимо ответить на следующие вопросы:
Можно сделать проще и удобнее?
Нет ли избыточности?
Соответствует ли задаче реализация?
Это костыль или нет?
Используются типовые решения или это новый велосипед?
Соответствует ли паттерну открытости/закрытости (если соответствует, то при доработках не потребуется переписывать все решение снова, а только добавлять новый код)?
Что смотрим при проведение инспекции кода?
Можно сформулировать в общем случае следующий набор вопросов на которые должен ответить инспектор кода:
Соответствует ли код стилю? Соблюдаются ли правила, стандарты и соглашения принятых в команде? (об этом ниже см. «что за стандарты и как их сделать?»)
Возникают ли у меня сложности в понимании?
Можно ли что-то исправить путем рефакторинга?
Хорошие наименования переменных, функций?
Есть ли излишнее усложнение кода? Высокая ли цикломатическая сложность (фактически на сколько большая вложенность если иначе)?
Соответствуют ли запросы критериям оптимальности?
Дополнительно еще можно составить некоторый чек-лист критериев проверки, на первых порах выполнять проверять в его рамках.
Что за стандарты и как их сделать?
Под стандартами необходимо понимать определенный набор правил, которые вы должны соблюдать и использовать в процессе разработки. Лучше сделать какой-нибудь документ с основными выдержками и ссылками на документы и стандарты. Вот что мы обсуждали:
1. Вы договариваетесь о правилах форматирования кода.
Порядок оформления комментариев, названия переменных, создания обработок, форматирования текста и т.п.
2. Вы договариваетесь о правилах доработки типовых конфигураций.
В нашем случае мы договорились о правилах минимального изменения конфигураций.
3. Используете правила и рекомендации по работе с БСП.
Нравится вам или нет, но к примеру, добавление нового присоединенного файла к производителям необходимо делать в соответствии с документацией от разработчиков.
4. Используете рекомендации по разработке от 1С.
5. Свои стандарты, правила и т.п.
Пример содержимого документа памятки:
А) Добавляем новые реквизиты и метаданные с префиксом ццц_
Б) Перед началом вставки кода и завершении добавляем специальные теги (используем шаблоны кода)
//++ Номер Задачи Автор
//— Номер Задачи Автор
В) Форматирование кода используем типовое
Г) Делаем коммиты на каждую задачу отдельно и при помещении в хранилище в комментарий добавляем метку с номером задачи
Д) и т.д. …
Кого проверяем? Всех или выборочно?
Ответ: Каждая задача, в которой было изменение кода или метаданных, должна пройти код-ревью. Соответственно задачи типа прочитать книжку по постижению дзена код-ревью не требуют)
Когда проводится код-ревью и как часто?
Ответ: Мы начинаем проведение ревью каждое утро с 9 часов утра, пока не закончатся задачи.
Либо проверка может выполняться по требованию в процессе рабочего дня (срочный баг фикс и т.п.).
И конечно по желанию.
Как проводится контроль?
Перевод в статус с код-ревью выполняет только ответственный по «код-ревью».
Как выгляди диалог общения внутри процесса ревью?
Ответ: Допустим только профессиональный диалог. Никаких личных отношений, переходов на «ты», высмеивания или пренебрежительного отношения.
Запомните цель code-review повысить качество вашего продукта, а не поднять настроение команде.
Правильный и не правильный процесс code-review
Что пишет автор перед отправкой на код-ревью?
Перед началом код-ревью автор решения должен подготовить список изменений, обозначить их в коде. Пример:
а) В задаче автор обязательно отмечает список измененных метаданных (еще несколько отдельных требований).
б) В хранилище конфигурации задачи обязательно помещаются с комментарием о номере задачи или номерах (желательно чтобы коммит был в рамках одной задачи).
в) В коде конфигурации подключенной к хранилищу разработчик отмечает начало изменения и конец изменения. Согласно стандарта (см. выше)
// ++ Задача-6800 Иванов
// комментируются данные до
// Если Объект.УчитыватьНДС Тогда
Если Объект.УчитыватьНДС и Объект.ццц_Учитывать
….
//— Задача-6800 Иванов
г) Если ведется разработка в EDT, то на код ревью отправляется номер ветки. Скажу честно, то в EDT сравнивать очень непривычно.
Как боретесь с «индивидуальными» (вкусовщиной) подходами?
Ответ: Есть общий стандарт и договоренность внутри команды. Все остальное вне закона.
Иными словами, если приходит задача и она не соответствует стандарту, то задача «сразу» отправляется в возврат.
Что писать в замечаниях к код-ревью?
Все замечания мы пишем в рамках тега «[КОД-РЕВЬЮ]» в нумерованном списке. Соответственно, автор при исправлении и объяснении своей позиции будет отвечать на каждый пункт из пронумерованного списка.
Если нет замечаний, то ставится резюме – «ОК». (пример оформления переписки см. пункт «Процесс проведения код ревью. Как проводится контроль?»)
Подтверждаем сове мнение примерами. В рамках проведения ревью будет правильно привести пример, на то как это было бы сделать правильно – особенно важно для новичка или джуна.
Делаем ссылки на стандарты или правила. Все свои рекомендации, версии должны быть подкреплены доказательствами. Фразы не допустимы: «Я так считаю», «Мне нравится» и т.д.
Соответствие стандартам. Если не соответствует стандартам так и пишите: не соответствует стандартам.
Какие есть автоматизированные инструменты?
Здорово если бы для 1С появился мощный статический анализатор кода типа PVS-studio, но пока его нет.
Про остальное не знаю. Внедрять и поддерживать полу-функциональные инструменты не считаю эффективным подходом.
Как происходит проверка технически?
Сколько времени тратится на код-ревью?
Мы рекомендательно ограничиваем время на проведение процедуры код-ревью 15-20 минут на задачу. Однако на этот временной параметр влияют множество факторов: уровень автора (эксперт или джун), сложность задачи, риски и последствия плохого исполнения задачи (фактор риска), возможная эскалация и призыв коллективного разума и т.д.
Как давно применяете?
Уже очень давно, минимум 5-6 лет.
Какие трудности возникают?
Их много особенно в начале пути.
В процессе проведения ревью довольно сложно проверять очень большие задачи по объему, изменения в которых затрагивают большой пласт кода и/или большое количество метаданных. В данном случае изначально рекомендуется разбивать большие задачи на маленькие.
Тяжелее проверять задачи в EDT, не смотря на наличие ветвлений. Тут проблемы технического характера, а также самого IDE. Разработчики от компании 1С обещали поправить, ждем.
Возникает борьба с уложившимися подходами новых разработчиков. Стандарт понимают, но сложившиеся годами навыки дают о себе знать.
Приходится всем следовать стандартам. Иногда так хочется запилить «по-быстрому», но нельзя.
Сам процесс внедрения и улучшение процесса вызывает определенные «сложности». Это, на мой взгляд, логично и понятно. А изменения должны быть, т.к. время течет и все меняется)
Заставить проводить код-ревью утром в понедельник). Поэтому мы ненавязчиво напоминаем коллегам об обязанностях.
Как повысить уровень, коммуникацию, эффективность внутри команды?
Для этого рекомендую проводить краткие вебинары (15-30 минут). Сейчас организовать подобные мероприятия не проблема – есть Skype, MS Teams, WebEx и др.
В первой части проводится ретроспектива и каждый из участников отвечает на три вопроса:
Что было плохого?
Что было хорошего?
Какие есть предложения?
Во второй части проводим обезличенное обсуждение найденных косяков, интересных находок, мастер классы и т.п.
На каких проектах целесообразно применять код-ревью?
Практически на всех. Не забывайте, что даже эксперт может «накосячить».
Излишне, наверное, будет инспектировать код при разработке прототипа и минимально жизнеспособного продукта (обработки, отчета на раз). Хотя и тут если особенно исполнение хромает, то лучше одним глазком посмотреть.
