Заставить ВСЕХ разработчиков делать обзоры кода

13

Я разработчик программного обеспечения в команде разработчиков 7-8. Мы уже давно занимаемся обзорами кода, и качество кода со временем улучшилось.

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

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

Как вы решаете эту проблему в своей команде?

Вы установили правила выбора рецензентов кода?

Как вы думаете, рецензенты кода должны быть вознаграждены за время, потраченное на выполнение (хороших) рецензий кода? И как они должны быть вознаграждены?

Спасибо за ваши ответы / идеи.

guillaumegallois
источник
7
Рассматривали ли вы создание круговой системы, в которой кодер остается в неведении относительно того, кто рецензирует, а рецензент остается в неведении относительно того, кто является кодером?
Нил
1
У меня нет, но мне нравится эта идея! Благодарность!
Гийомегаллойс
1
Кто отвечает, и почему они не выполняют свою работу, которая должна включать обеспечение того, чтобы все остальные выполняли свою работу?
JeffO
В моей команде рецензенты автоматически назначаются при каждом открытии запроса на включение. Рецензенты отбираются из команды Round-Robin. У нас есть webhook для каждого из наших репо, который автоматически назначает рецензентов при каждом открытии PR. Он в основном хранит список всех разработчиков и тех, кто был последним назначен, и просто работает через этот список.
Дэн Джонс

Ответы:

12

Мы не выбираем рецензентов.

В нашей команде:

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

Обзоры кода не назначаются, люди выбирают их, когда это работает для них. Понимание состоит в том, что мы не можем протолкнуть истории через конвейер. Иногда кто-то упоминает, что он ожидает КР в режиме ожидания, но это все.

Мне нравится эта модель, она дает людям возможность выбрать то, что они могут, и избегает "давать людям работу".

Liath
источник
6

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

Что касается,

Как вы думаете, рецензенты кода должны быть вознаграждены за время, потраченное на выполнение (хороших) рецензий кода?

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

max630
источник
2
Это интересная идея, но во многих случаях, когда возникает ошибка, 90% работы выясняют, что именно является причиной ошибки, и 10% работы исправляют ее. Выполнение вскрытия, чтобы выяснить, какие именно изменения привели к ошибке, может даже не произойти, если только это не поможет выяснить, что происходит, или как сделать безопасное исправление.
DaveG
Вы хорошо отметили, что рецензентам кода следует отдать должное. Это определенно проблема, которую нужно решать. Спасибо за Ваш ответ!
Гийомегаллойс
Я думаю, что это может заставить людей вообще не пытаться делать обзоры кода, или может быть, что у вас не будет никакой работы, потому что люди начнут придираться.
Матеуш
5
Этот ответ предполагает, что целью проверки кода является поиск ошибок. Это не так, основная цель - сохранить код в поддерживаемом и развивающемся состоянии (и, если вам повезет, обнаружатся и некоторые ошибки).
Док Браун
@DocBrown для предотвращения ошибок, а также для обеспечения более быстрого исправления ошибки в будущем (как путем ознакомления кода с другим разработчиком, так и с гарантией того, что код хорошо написан)
max630
4

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

  • Понимание того, что это за изменение. Запросы Git Pull для ветвей функций действительно помогают этому процессу.
  • При создании кода просмотрите событие, когда люди должны находиться в одной комнате. Это добавляет нагрузку от планирования, ресурсов для собраний и т. Д. GitHub, GitLab и BitBucket позволяют выполнять асинхронные проверки, чтобы они могли происходить, когда партнер готов.
  • Возможность обеспечить значимую обратную связь при просмотре кода. Честно говоря, функция построчного комментирования в GitHub, GitLab, Bitbucket pull-запросах действительно более полезна, чем встреча лицом к лицу. Это чувствует себя менее политическим.

Это не означает, что вы не можете использовать SubVersion или другие инструменты (например, Fisheye), чтобы помочь, но интеграция, встроенная в конвейер Git с ветвями функций, действительно делает работу менее трудоемкой.

Помимо инструментов, вам нужно взглянуть на более социальные проблемы:

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

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

Берин Лорич
источник
Последний пункт хорош. Однажды я работал с разработчиком в небольшой команде, которая только когда-либо проверяла изменения в программном обеспечении, которые он написал, что вызывало значительные узкие места в других частях команды.
Робби Ди
В таком случае звучит так, будто у вас есть кто-то, кто пытается защитить свою «территорию». Насколько это возможно, вы хотите развить чувство долевого участия в коде. Другими словами, в коде нет защищенных святилищ, которых не могли бы коснуться другие разработчики, кроме «благословенных». Я понимаю, что если есть пробел по специальности, и вы не можете просмотреть математику, но вы можете, по крайней мере, проверить, насколько хорошо код соответствует его цели.
Берин Лорич
2

Хорошая идея - сделать это на основе циклического перебора - вы выбираете кого-то, кто выполнил наименьшее количество проверок вашего кода. Со временем все в команде должны были сделать примерно равное количество обзоров. Это тоже распространяет знания.

Очевидно, что будут случайные исключения, такие как праздники, где будут пики и впадины.

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

Если после всего этого вы по-прежнему обеспокоены качеством проверок, рассмотрите возможность введения набора минимальных стандартов для прохождения проверки кода. То, что вы включаете, полностью зависит от вас, но некоторые вещи, которые вы можете рассмотреть, это: покрытие кода, модульные тесты, удаление закомментированного кода, метрики, достаточное количество комментариев, качество сборки, принципы SOLID, DRY, KISS и т. Д.

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

Робби Ди
источник
0

Похоже, команде не хватает формального процесса проверки кода.

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

Важные биты:

  1. Определите основной набор рецензентов. Нет общих заявлений. Назовите людей.

    Это должны быть ваши старшие разработчики.

  2. Требовать более одного основного рецензента, чтобы подписать рецензию.

  3. Определите по крайней мере 1 другого не основного рецензента на каждый спринт или релиз, который является временным основным рецензентом. Требуйте их подписи на всех кодах отзывов за это время.

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

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

Если вы сомневаетесь, определите процесс и скажите команде, что он должен придерживаться его.

И есть одна последняя вещь, которую вы можете попробовать - спорным, так как это может быть: пусть @ # $% ударил вентилятор, если я могу использовать идиомы.

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

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

Иногда вам просто нужно, чтобы Титаник ударил ледяной бург.

Грег Бургардт
источник