Agile Practices: проверка кода - провалить проверку или поднять проблему?

53

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

В противном случае задача соответствует определению выполненного.

У нас есть два варианта.

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

У меня вопрос: есть ли какие-то проблемы или соображения, связанные с поднятием заявки с обратной стороны обзора, а не с ошибкой?

Ресурсы, которые я могу найти и которые читают подробные обзоры кодов, обычно составляют 100% или ничего, но я считаю, что это обычно нереально.

Эрдрик Айронроуз
источник
23
Итак, если вы не можете провалить проверку кода для этого, какова цель обзора? Прямо сейчас вам кажется, что стоит посмотреть, работает ли что-то , однако, безусловно, это работа теста или тестера, а не проверка кода.
ВЛАЗ
21
Я думаю, что в большинстве ответов отсутствует важный момент в вашем вопросе: в противном случае задача соответствует определению «выполнено». Являются ли упомянутые вами проблемы частью того, что ваша команда считает «не выполненным»? Или эти вопросы не рассматриваются в том, что должно быть выполнено? Если ваше определение «выполнено» включает «отсутствие запаха кода», то задача просто не выполнена.
Джош Парт
1
@ErdrikIronrose, так что это звучит так, как будто изменение не соответствует стандартам и, возможно, не (так легко) обслуживаемо. Хотя ваш другой комментарий, похоже, указывает на то, что изменение не было частью запроса, в этом случае оно не должно быть частью проверки кода. Если кто-то пишет правильный и стандартный код рядом с существующим уродливым хаком, то не стесняйтесь, подайте заявку на исправление этого уродливого хака и пройдите текущий обзор кода. Если кто-то пишет код, который является правильным, но не соответствует стандарту (как показывает ваш вопрос), то не завершайте проверку кода, пока она не будет выполнена правильно.
ВЛАЗ
9
@ErdrikIronrose: Ах, так как запах кода не был создан при работе над рассматриваемой историей , но уже существовал? Это важное различие - подумайте над редактированием вопроса.
слеске
1
@vlaz ты должен ответить на свой комментарий
Истер

Ответы:

67

Есть ли какие-либо проблемы или соображения, связанные с поднятием заявки с обратной стороны обзора, а не с ее провалом?

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

в обзоре мы обнаруживаем функцию

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

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

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

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

Если это так, то у вас смешанные приоритеты. Стандарт чистого кода не следует менять просто потому, что он делает команду счастливее. Правильность и чистота кода не зависят от настроения команды.

Рефакторинг - это небольшая часть работы, которая будет выполнена в следующем спринте (или даже до того, как он начнется) в виде крошечной истории с половиной баллов.

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

Ресурсы, которые я могу найти и которые читают подробные обзоры кодов, обычно составляют 100% или ничего, но я считаю, что это обычно нереально.

Проход / неудача по своей сути является двоичным состоянием , которое по своей сути является всем или ничем.

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

Код не должен быть безупречным, он должен просто соответствовать разумным стандартам чистоты, применяемым вашей командой / компанией. Приверженность этому стандарту является бинарным выбором: он придерживается (проходит) или нет (не проходит).

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

В противном случае задача соответствует определению выполненного.

Если бы «он выполнял свою работу» был лучшим эталоном качества кода, то нам не пришлось бы изобретать принцип чистого кода и хорошую практику для начала - компилятор и модульное тестирование уже были бы нашим автоматическим процессом проверки и Вам не понадобятся обзоры кода или аргументы стиля.

Flater
источник
26
«Правильность и чистота кода не зависят от настроения команды». +1 только для этого, однако единственным предостережением для всего этого ответа будет крайний срок. Если провал этого обзора кода означает, что долгожданная функция не попадет в следующую версию, вы должны сбалансировать чистоту кода с потребностями клиента. Но помните, что неправильный код, который соответствует крайнему сроку клиента сегодня, завтра станет производственным вопросом.
Грег Бургхардт
11
Отличный ответ - твердый, но не грубый. Также может возникнуть одна тангенциальная точка: как нам удалось сделать так, чтобы проверки кода происходили так поздно в спринте, что простой рефакторинг не мог быть выполнен без провала всего спринта?
Даниил
@ Дэниел: Разработчик может быть вовлечен иным образом, или это может быть проблема планирования. Время между завершением задания и завершением спринта обычно минимально, так как (в идеальном мире) люди заканчивали бы свою последнюю задачу спринта во время закрытия спринта. Вы не можете взять длительный период, чтобы рассмотреть / исправить; или в качестве альтернативы, возможно, разработчик просто отсутствует / доступен для остальной части спринта.
Флатер
8
+1 Программисты могут чувствовать себя хорошо, когда они написали хороший код. Обход контроля качества не является ответом на улучшение морального состояния. В любом случае, случайный отказ от мелких проблем вряд ли заставит морально пострадать. Если ваш моральный дух страдает из-за того, что вы регулярно не проходите контроль качества, ответ заключается в том, чтобы постоянно что-то делать с отказом от контроля качества, а не отказываться от стандартов.
jpmc26
1
@GregBurghardt: Поскольку я видел злоупотребление аргументом о крайнем сроке во многих компаниях, я склонен согласиться на плохой отзыв только тогда и только тогда, когда задача для его немедленного рефакторинга создана и запланирована для первого спринта после релиза. Добавленная стоимость времени добавляет значительный барьер входа для обхода качества кода.
Флейтер
38

По окончании 2-х недельного спринта и задания есть проверка кода [...] Легкая работа по рефакторингу.

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

Если вы заканчиваете рассказы так незадолго до просмотра демонстрации / спринта, что не можете выполнить это «крошечное» задание, тогда вам нужно лучше оценивать свою работу. Да, эта история не закончена. Не из-за проверки кода, а потому, что вы не планировали вносить изменения из проверки кода. Это все равно что оценивать «тестирование» на нулевое время, потому что «если вы запрограммировали его правильно, оно просто сработает, верно?». Это не так, как это работает. Тестирование обнаружит ошибки, а проверка кода найдет, что изменить. Если этого не произойдет, это будет большая трата времени.

Итак, подведем итог: да, DoD является двоичным. Проход или провал. Обзор кода не является двоичным, он должен быть больше похож на текущую задачу. Вы не можете потерпеть неудачу . Это процесс, и, в конце концов, это сделано. Но если вы не спланируете должным образом, вы не доберетесь до стадии «выполнено» вовремя и застрянете на «не выполненной» территории в конце спринта. Это не хорошо для морального состояния, но вы должны учитывать это при планировании.

nvoigt
источник
5
Это именно тот ответ, который пришел мне в голову. Если каждая история реализуется со своей собственной веткой, то не откладывайте просмотр и объединение ветвей до конца спринта. Вместо этого создайте запрос на извлечение, как только считается, что ветвь готова, и продолжайте итерацию в этой ветке, пока она не будет фактически выполнена, утверждена и объединена. Если этот процесс не закончился в конце спринта, то история еще не закончена.
Даниэль Приден
20

Просто: вы просматриваете изменения . В противном случае вы не просматриваете состояние программы. Если я исправлю ошибку в функции 3000 строк, то вы убедитесь, что мои изменения исправят ошибку, и все. И если мое изменение исправляет ошибку, вы принимаете это изменение.

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

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

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

gnasher729
источник
4
Я думаю, что в этом случае изменение было чрезмерно длинной функцией - если вы ввели функцию 3000 строк, которой раньше не было (или ранее была функция 10 строк).
user3067860
3
В принципе этот ответ является абсолютно правильным. На практике ..... Если все разработчики верят и практикуют хорошие методы кодирования, сбалансированные с усилиями, то вы, вероятно, не будете сталкиваться с этой проблемой очень часто, и тогда этот ответ на месте. Однако ... кажется, что всегда есть один или два разработчика, которые делают все быстро и грязно, чтобы сэкономить 5 минут; в то время как они игнорируют часы или дни или месяцы, которые они добавляют к работе, которая будет позже. В этих случаях этот ответ - всего лишь скользкий путь к необходимости начинать все сначала и перепроектировать всю систему.
Данк
+1, хотя я думаю, что вам следует перефразировать последний абзац, чтобы выделить, что проверка кода с проблемами должна быть абсолютным исключением. Я имею в виду, просто то, что кто-то заблокирован, не достаточно оправдания. Провал одного спринта тоже не выглядит достаточным оправданием, и уж точно не может быть оправданием, которое можно использовать повторно.
Frax
@ user3067860 Если вы превратили функцию с 10 строками в функцию с 3000 строками - тогда явно произойдет сбой. Если вы превратили функцию 3000 строк в 3010 - тогда, вероятно, пройти. Но что если вы превратили функцию из 100 строк (обычно слишком большую) в функцию из 300 строк ( определенно, слишком большую)?
Мартин Боннер поддерживает Монику
9

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

Кажется, это проблема.
Теоретически вы знаете, что должны делать, но это близко к крайнему сроку, поэтому вы не хотите делать то, что знаете, что должны делать.

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

xyious
источник
«Уважаемый клиент, у вас не может быть вашей функции еще 2-3 недели, потому что наш код работал, но нам не понравилось, как он выглядел», ... пожалуйста, не переходите к нашему конкуренту ... или не говорите генеральному директору !
RandomUs1r
6
Клиенты @ RandomUs1r не должны иметь такой информации. Это не было сделано, потому что не было достаточно времени для этого и все. Заказчики диктуют, как код должен быть написан? Если вы звоните электрику, чтобы починить проводку у себя дома, вы говорите: «Просто замените кабели, но не проверяйте, правильные ли это кабели»? Или вы говорите своему врачу: «Я болен - дайте мне несколько таблеток, но сначала не ставьте мне диагноз»? Проверка кода должна быть неотъемлемой частью работы, а не тем, что диктует заказчик.
ВЛАЗ
1
@ RandomUs1r: «« Уважаемый разработчик, почему функция не была завершена? » - ответ должен быть« потому что у нас не было достаточно времени, чтобы построить его до приемлемого уровня качества », возможно, за ним следует« Мы можем дать его вам, если вы готовы пойти на компромисс по качеству ".
Брайан Оукли
1
@ RandomUs1r, так что в основном вы хотите пожертвовать качеством кода, что, вероятно, усложнит реализацию функций позже. 2-дневное исправление теперь вполне может сэкономить вам 4-недельное исправление позже. Итак, «Уважаемый клиент, вы не можете использовать свою функцию в течение еще двух-трех недель, потому что для реализации второстепенной функции требуется так много времени». Также это конец спринта или это крайний срок? Если это большой срок, я мог бы увидеть слияние сейчас, написание исправления в течение следующих 2 дней и повышение PR сразу после крайнего срока.
xyious
5
Все, что я говорю, это то, что если ваши стандарты отличаются в первый день и последний день спринта, то у вас нет стандарта, и ваше качество неизбежно ухудшится.
18
5

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

Когда дело доходит до проблем с проверкой кода, есть несколько способов справиться с этим, и правильный выбор зависит от нескольких факторов.

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

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

Берин Лорич
источник
4

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

  1. Какова цель вашего обзора кода?
  2. Как результаты проверки кода соотносятся с определением «Готово» для рабочего элемента?
  3. Если проверка кода применяется в качестве теста на стробирование, какие проблемы считаются «блокирующими»?

Все это сводится к тому, что команда согласилась с определением «Готово». Если прохождение проверки кода с нулевыми ошибками является определением выполненного для рабочего элемента, вы не можете закрыть элемент, который не соответствует этому требованию.

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

Если команда не согласилась с тем, что проверки кода являются определением «Готово», то проверки кода не являются проверочным приемочным тестом для рабочего элемента. Это командная деятельность, которая является частью вашего процесса ожидания, чтобы найти дополнительную работу, которая может потребоваться. В этом случае любые обнаруженные вами проблемы не связаны с требованиями исходного рабочего элемента и являются новыми рабочими элементами, которые команда должна расставить по приоритетам.

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

Джей С
источник
1

Ответы с более высоким рейтингом здесь очень хороши; это касается угла рефакторинга.

В большинстве случаев большая часть работы по рефакторингу заключается в понимании существующего кода; изменение его после этого - обычно меньшая часть работы по одной из двух причин:

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

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

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

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

Рефакторинг - это небольшая часть работы, которая будет выполнена в следующем спринте (или даже до того, как он начнется) в виде крошечной истории с половиной баллов.

В этом случае вы думаете о создании отдельной истории для рефакторинга, это предупреждающий знак по нескольким направлениям:

  1. Вы не думаете о рефакторинге как о части кодирования, а как о отдельной операции, которая, в свою очередь, делает возможным падение под давлением.

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

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

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

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

Курт Дж. Сэмпсон
источник
1

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

Ваш вопрос явно призывает к «гибкой практике», поэтому давайте вернемся к гибкому манифесту (выделение мое):

Мы раскрываем лучшие способы разработки программного обеспечения, делая это и помогая другим делать это. Благодаря этой работе мы пришли к оценке:

  • Индивидуумы и взаимодействия над процессами и инструментами
  • Рабочее программное обеспечение над полной документацией
  • Сотрудничество с клиентом в рамках переговоров по контракту
  • Реагирование на изменения после плана

То есть, хотя в элементах справа есть ценность, мы слева оцениваем элементы больше.

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

Начни сотрудничать. Хватит проваливать проверки кода.

Муравей П
источник
Я все за сотрудничество. Но какой термин вы бы использовали, если бы не «провал»? Даже обсуждая, как группа, один человек сказал бы, что «это недостаточно хорошо, оно нуждается в рефакторинге», что просто означает, что он не прошел проверку качества, верно?
Эрдрик Айронрос Роуз
1
@ErdrikIronrose Я никогда не использовал и не нуждался в использовании терминологии «провал» обзора кода. Кто-то просматривает код, после чего обсуждается вопрос о возможных улучшениях, а затем принимается решение о том, следует ли решать эти вопросы. Здесь нет «прохождения» или «провала», просто общение и прогресс. Я не уверен, зачем нужен резиновый штамп.
Муравей P
0

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

Есть ли какие-то проблемы или соображения, связанные с поднятием заявки с обратной стороны обзора, а не с ошибкой?

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

Одно из высказываний, которое мы имеем: «Выбирай прогрессивное улучшение вместо отложенного совершенства».

У нас очень плавный процесс, и мы создаем довольно много функций «проверки концепции» (1 или 2 на спринт), которые проходят через разработку и тестирование, но никогда не проходят проверку внутренними заинтересованными сторонами (хм, можем ли мы сделать это вместо этого? ?), альфа или бета ... некоторые выживают, некоторые нет.

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

railsdog
источник
0

Есть два способа взглянуть на эту проблему по моему мнению:

  1. Академический путь
  2. Путь реального мира

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

Тем не менее, никто в реальном мире не придерживается гибкого подхода к Т, так как по разным причинам многие отрасли предъявляют разные требования. Таким образом, исправление кода сейчас или взятие на себя технического долга (вы, скорее всего, создадите новый PBI) должны решаться для каждого конкретного случая. Если это может скомпрометировать спринт или релиз, или привести к необоснованному риску, заинтересованные стороны должны быть вовлечены в решение.

RandomUs1r
источник
2
никто в реальном мире не следует за Agile T - это больше не будет Agile, если у нас слишком строгие правила, верно?
Пауло Эберманн
@ PaŭloEbermann У меня был забавный разговор с компанией, с которой я однажды беседовал. Они утверждали, что их процесс не был гибким, потому что это не был пример гибкого учебника. Хотя все, что они делали, было в духе проворной. Я указал им на это, но был встречен только (по существу): «Нет, мы не следуем установленной гибкой процедуре до буквы, даже если мы сильно заимствуем понятия. Поэтому мы не проворны». Это было довольно странно.
ВЛАЗ
Как отмечали другие рецензенты, в этом случае потенциально можно извлечь урок из-за неспособности кода действительно пройти рецензию. Мне кажется, что люди в этом проекте действительно не очень хорошо понимают, что а) вам нужно оставить время для обзора и исправлений для каждой истории, и б) рефакторинг, необходимый для того, чтобы оставить чистый код, является неотъемлемой частью сказка. В этом случае лучше всего провалить историю, чтобы понять, что эти вещи на самом деле не являются обязательными.
Курт Дж. Сэмпсон
@ Курт Я понимаю, что мое мнение может быть непопулярным с точки зрения разработчика (я, кстати, тоже разработчик), но бизнес действительно должен быть на первом месте, они подписывают зарплату, и это заслуживает некоторого уважения. Что касается уходящего времени, я снова буду оспаривать ваше понимание реального мира, и вам нужно осознать, что это не всегда возможно, и многие спринты не работают, потому что разработчикам тоже нужно что-то делать в конце спринта. Это не так, потому что код SOLID: департамент может подняться на 1/10 дня каждые 2 недели и ничего не делать, что может быть замечательно в краткосрочной перспективе, но не жизнеспособно долго.
RandomUs1r
@ RandomUs1r Я тоже работаю в реальном мире, все время использую ярлыки и всегда ставлю дело на первое место, поэтому я не думаю, что мне не хватает понимания здесь. Но описание ОП не было «мы обычно всегда понимаем это правильно, а это была просто небольшая ошибка», иначе он не стал бы публиковать этот вопрос. Как я объяснил в своем ответе, это похоже на проблему процесса, и вы решаете ее, практикуя правильное выполнение процесса, прежде чем расслабиться с ним.
Курт Дж. Сэмпсон
-2

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

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

  1. «Функция довольно длинная». Запишите: функции должны быть длиной менее X строк (я не предполагаю, что правила о длине функции - это хорошо).

  2. «Есть некоторые запахи кода». Запишите: публичные функции должны иметь модульные тесты на функциональность и производительность, использование ЦП и памяти должно быть в пределах x и y.

Если вы не можете количественно определить правила для прохождения проверки кода, то вы получите эти примеры того, что по сути является «кодом, который вам не нравится».

Если вы провалите «код, который вам не нравится»? Я бы сказал нет. Вы, естественно, начнете проходить / не проходить, основываясь на не кодовых аспектах: вам нравится человек? Они решительно отстаивают свое дело или просто делают, как им говорят? Они передают ваш код при просмотре?

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

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

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

Re: невозможно записать правила!

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

Запишите правила, которые вы можете, и обсудите с пивом, что делает код «хорошим».

Ewan
источник
6
Нет, вы упускаете мысль о том, что «иметь идеальный и универсально применимый стандарт без двусмысленностей» не является реалистичным предварительным условием для анализа кода. Всегда будут возникать новые типы проблем, которые вы еще не учли, и, следовательно, вам необходимо принимать решение на неизведанной территории. Конечно, вы должны затем задокументировать это решение, чтобы оно больше не являлось неизведанной территорией, но ваш ответ основывается на предположении, что вы можете каким-то образом гарантировать отсутствие неизведанной территории, если только вы подготовите идеальные правила перед рассмотрением. Вы кладете телегу перед лошадью.
Флатер
5
Абсолютные слова типа «функции должны быть длиной менее x строк» также не являются ответом .
Blrfl
2
Договорились с Blrfl. Функции (в общем) не должны превышать 20 строк. Но сделать это абсолютным правилом - ошибка. Конкретные обстоятельства всегда берут верх над общими правилами: если у вас есть веская причина сделать свою функцию более 20 строк, сделайте это.
Мэтт Мессерсмит
1
Вам не нужны правила для кода, написанного в юридической спецификации ... У вас могут быть только руководящие принципы плюс тот факт, что все вы, вероятно, взрослые, которые пытаются достичь одной и той же конечной цели (рабочий, читаемый, поддерживаемый код). Все искренние инвесторы в команду и желание работать вместе - это главное для Scrum, так что если у вас его нет, то, возможно, Scrum не для вашей команды.
user3067860
2
@ Иван Конечно. Моя точка зрения заключалась в том, что у ОП есть ориентир по длине функции , а не правило. Везде, где установлен порог, он дает советы, которые помогают людям находить трудно поддерживаемый код, но это никогда не является абсолютным правилом. Если (как говорит OP) он действительно отлично читается, и рецензенты согласны с тем, что он отлично читается, и нет проблем с его надлежащим тестированием, тогда функция по определению имеет правильный размер. В обзоре может быть записка в одну строку с надписью «Да, это дольше, чем рекомендовано, но мы согласны, что все в порядке», и работа выполнена. Рефакторинг после этой точки - позолота.
Грэм