Как эффективно отслеживать рецензирование кода?

28

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

Мне кажется, что нет такого понятия, как проверка кода без единого комментария.

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

Обновить

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

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

Поэтому я добавил библиотеку с именем «jscpd» для обнаружения копий. Сбой сборки на копирующих пастах. Это сразу устранило одну проблему.

Далее мы собираемся попробовать кодеклимат.

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

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

парень Мограби
источник
1
В случае, если вы используете TFS, вы можете настроить его на включение имени Code Reviewer.
Кришнанду Саркар
11
@gnat Я не согласен. Есть разница между тем, кому не нравятся обзоры кода и о чем спрашивает этот вопрос. Этот вопрос может быть атакован с точки зрения прослеживаемости (связывание изменений в исходном коде с обзором или дефектов / улучшений / историй с обзорами этой реализации и т. Д.) Или с точки зрения качества процесса и аудита. Оба имеют значение, даже если у людей обычно нет проблем с проверкой кода.
Томас Оуэнс
3
Вы посещаете какие-либо из этих обзоров? Может пора заскочить на одного? Укажите на себя несколько вещей и спросите каждого рецензента в отдельности, почему он пропустил их все?
Mawg
2
Считаете ли вы, что очевидные проблемы не были обнаружены в обзоре? Бы вы добавили (важные) комментарии?
USR

Ответы:

70

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

Но по моему опыту, я подозреваю, что происходит что-то еще.

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

Так что в ваших ретроспективах (если вы проворны) или один на один (если вы менеджер) или в случайных импровизированных встречах в коридоре (если вы не лидер команды, а другой менеджер делает один на один), поднимите это , Спросите, что люди думают о процессе проверки кода. Как это работает? Как это не так? Скажем, вы думаете, что это может принести команде не столько пользы, сколько могло бы. Убедитесь, что вы слушаете .

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

Использование инструмента поверх сломанного процесса не улучшит процесс.

Telastyn
источник
5
+1 за правильный подход к этой (и многим другим!) Проблеме
Оливье Дюлак
7
+1 за последнее предложение. Это то, что почти никто не понимает, но чрезвычайно важно.
JohnEye
1
Хороший ответ. Попробовал это ... Моя команда говорит, что "компания делает вещи неправильно. Нам нужно больше QA ... и позволить разработчикам развиваться", в то время как компания говорит "Мы хотим, чтобы разработчики представляли хороший код качества. Мы стремимся разогнать команду QA". поскольку, как только код в хорошем качестве, qa больше не нужны ... "... В конечном итоге произошло то, что люди, которые получали плохой код, постоянно увольнялись, и я восстанавливал свою команду.
парень mograbi
43

Мне не нравится публиковать однострочные ответы, но этот кажется подходящим:

Участвуйте в процессе.

Blrfl
источник
15
Мне также не нравятся однострочные ответы. К счастью, вы взяли две строки - и мой ответ. +1
Мауг
1
Я. но когда я не .. вещи случаются. это именно то, что сделало меня подозрительным в первую очередь. Я начал пересматривать чужой отзыв и обнаружил неприятные вещи.
парень mograbi
6

Получить инструмент, такой как ReviewBoard или плагин Redmine Codereview . Затем каждый отзыв создается как задача, которая должна быть закрыта или прокомментирована кем-то (как ошибка). Затем вы можете отследить, кто создал тикет обзора, а кто закрыл его. Вы можете связать проверочные заявки с проверками исходного кода, то есть создать заявку из ревизии.

gbjbaanb
источник
2

Несколько вещей (честно говоря, большинство из них описаны в ответах, но я хотел поместить их в одном месте)

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

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

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

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

Мэтт Фрейк
источник
2

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

  • Убедитесь, что код выполняет то, что должен делать, т.е. отвечает требованиям

  • Стиль кода, чтобы гарантировать, что разработчики кодируют в единый стиль

  • Оптимизация, например, количество вызовов функций

  • Архитектура и возможность повторного использования

  • Обработка исключений и ведение журнала

  • Технический долг: код в лучшем состоянии, чем когда разработчик начал над ним работать

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

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

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

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

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

br3w5
источник
это реклама для SonarQube? Я попробовал это - я не рекомендовал бы это, слишком болезненный, чтобы начать и в то время как "открытый исходный код" стоит для всех полезных битов.
gbjbaanb
Он отлично работает в моей нынешней команде и не слишком сложен в настройке, и это помогает - это не реклама, а единственный инструмент такого рода, с которым у меня есть опыт. Вы бы сказали то же самое для Redmine Codereview и ReviewBoard?
br3w5
Мы используем SonarQube в наших командах, обслуживая около 70+ проектов, в диапазоне от 10 тыс. До 3 млн. LOC. Хотя некоторые команды просто игнорируют его отчеты, большинство используют его для управления процессами рефакторинга. Это работает хорошо, хотя лично я предпочитаю простые, не интегрированные инструменты, такие как Findbugs.
Диббеке
И тут я подумал, что при проверке кода проверяется, соответствует ли код проектному документу: - /
Mawg
1
Спасибо, это то, чем я занимаюсь. Я обновлю в течение нескольких недель, как это повлияло.
парень mograbi
0

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

Во-первых, позвольте мне задать вам вопрос. Используете ли вы систему контроля версий (например, Mercurial, Git)?

Если ваш ответ да, тогда продолжайте.

  1. запретить всем толкать что-либо (даже небольшие исправления) непосредственно в главную ветвь (транк) *
  2. разрабатывать новые функции (или исправления) в отдельных ветках
  3. когда разработчики считают, что ветка готова к интеграции в master, они создают «запрос на извлечение»
  4. запретить всем объединять свои собственные запросы *
  5. пусть другой разработчик оценит запрос на извлечение и рассмотрит новый код
  6. если код проходит проверку, хорошо, запрос на извлечение может быть объединен, в противном случае могут быть применены исправления.
  7. повторять шаг 6 до тех пор, пока код не станет достаточно зрелым (это можно сделать без повторного запуска)
  8. готово, весь ваш новый код проверен (хотя бы кратко) кем-то с именем

Теперь у вас есть точная точка в вашем рабочем процессе, где выполняется проверка кода.

Действуй там.

* может быть применено автоматически, с помощью серверных перехватчиков

** эта процедура полностью поддерживается GitHub (среди прочих) и довольно проста в использовании, ознакомьтесь с ней

Agostino
источник
2
Даже с таким процессом (который я предположил на самом деле происходит из описания в вопросе), у вас иногда есть разработчики, которые думают: «Ах, я достаточно доверяю своему коллеге и слишком много делаю сам, поэтому я просто сливаю его, фактически не читая» детали, или даже комментируя это ". (У нас похожий процесс в нашей команде с двумя необходимыми одобрениями (от людей, не являющихся автором PR), прежде чем его можно будет объединить. Тем не менее, иногда изменения проходят без тщательного анализа.)
Paŭlo Ebermann
1
@ PaŭloEbermann, я вижу. Боюсь, это неизбежный исход обстоятельств, если у вас не будет достаточно времени, чтобы сделать все, что вам нужно, качество пострадает, так или иначе. Конечно, если он не работает «иногда», значит, он работает «большую часть времени», нет?
Агостино
1
Да, это немного помогло, разрешив объединение только ограниченному кругу людей, у которых была задача проверить, был ли реальный обзор выполнен правильно.
Paŭlo Ebermann
У меня был аналогичный запрет, и само собой разумеется: разработка почти остановилась. Это правило длилось целых 2 недели, после чего руководителям приходилось корректировать свои планы.
BЈовић
@ BЈовић была ваша команда делает анализ кода на регулярной основе до того ? Этот метод используется многими, особенно в экосистеме Open Source. Тот факт, что это не сработало для вашей команды, не означает, что вы не можете работать на других.
Агостино
-2

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

user85
источник