Насколько важны положительные отзывы в обзорах кода?

48

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

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

Должен ли быть баланс между положительными и отрицательными отзывами?

c_maker
источник
3
Эй, если это пройдет, это положительный отзыв. :)
Адриан Дж. Морено
1
В большой степени, я думаю, это зависит от человека, чей код проверяется. Если они негативно отреагируют только на то, что получат критику, тогда важно найти баланс; в противном случае положительный отзыв является излишним, поскольку прохождение отзыва по своей сути положительно. Если они делают что-то новое и замечательное, вы можете упомянуть об этом, но включение этого в лучшие практики вашей команды также будет положительным откликом.
Мэтт
SE включает в себя как положительные, так и отрицательные голоса, поэтому положительные отзывы должны быть важны для правильной работы. Как бы это произошло, если лучшее, на что можно надеяться на ваши вопросы и ответы, - это ноль? Это стереотипное различие между мужчинами и женщинами: для мужчин отсутствие обратной связи означает «все в порядке». Для женщин это означает: «не было ничего хорошего, чтобы сказать». Это может очень долго объяснять относительную привлекательность этого поля для мужчин и женщин.

Ответы:

41

Улучшение качества и морального духа с помощью экспертных обзоров
http://www.slideshare.net/SmartBear_Software/improve-quality-and-morale-using-peer-code-reviews

Вещи, которые каждый должен делать: обзор кода
http://scientopia.org/blogs/goodmath/2011/07/06/things-everyone-should-do-code-review/

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

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

Роберт Харви
источник
26
Меня! Меня! Помимо сарказма, я на самом деле очень раздражен обзорами кода, где нет критики моего кода; Я знаю, что я не все делал идеально, поэтому отсутствие критики заставляет меня чувствовать, что я трачу свое время, даже прося пересмотреть. Поэтому я согласен с тем, что ничего, кроме критики, не плохо, но и это не так.
Джимми Хоффа
3
Я склонен согласиться с Джимми Хоффа. В целом (не только в обзорах кода), я нахожу очень раздражающим иметь дело с людьми, которые пытаются получить много положительных отзывов. Положительные отзывы должны быть полезны: нет необходимости загрязнять рецензию словами, которые автор кода уже знает. Лично я предпочитаю отношение: «Вы великолепны и умны, но в вашем коде есть несколько незначительных проблем».
Арсений Мурзенко
6
@MainMa: « Хорошо выглядит» работает для меня, если проблем не найдено. Для особенно полезного или хорошо написанного кода: «Это может быть полезно. Давайте разместим его в нашем архиве обмена кодами с некоторыми примечаниями или попробуем включить его в нашу повседневную практику кодирования».
Роберт Харви
2
Однажды кто-то бросил курить из-за того, насколько ужасными были наши обзоры кода. С тех пор мы перешли на использование обзоров кода в качестве практического семинара с небольшой критикой кода за серьезные проблемы, но в основном для образования. Парень, который ушел, попал в кричащий матч с нашим менеджером во время обзора из-за расхождений во мнениях. Люди могут проявлять действительно защитную реакцию во время обзоров, поэтому я настоятельно рекомендую давать положительные отзывы, чтобы снять напряженность и сделать моменты «вы должны изменить это» легче для рецензируемого, если не по какой-либо другой причине, кроме случайного нанесения удара эго
Брайан
2
@ Брайан, я должен сказать, что если кто-то попадет в кричащий матч из-за небольшой критики, он, скорее всего, будет отвлекать внимание от корпоративной культуры и морального духа офиса, я думаю, вам лучше.
Джимми Хоффа
29

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

Я думаю, что таким образом это не «оо ля ля, это так здорово!», Это может быть совершенно тривиальный скучный код, но слышать, как кто-то на самом деле анализирует и показывает понимание того, что вы написали, является формой положительной обратной связи самой по себе, обратная связь звучит так: «Этот код имеет смысл», когда я сталкиваюсь с частями, которые мне не понятны, я прошу объяснений, а когда я все понимаю, я восклицаю: «Ах, я понял».

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

Тем не менее, если вы видите части кода, которые имеют хорошие или положительные характеристики (даже скучный тривиальный код может быть хорошим, если он сам по себе является минимальной формой), я определенно склонен утверждать эти характеристики, опять же, я не приписываю их как «Wow здорово!" так же, как «Я вижу, что это минимальная реализация» или «Хорошо, этот сложный алгоритм имеет много комментариев», сосредоточьтесь на атрибутах кода, а не на его достоинствах или недостатках.

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

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

«Я вижу, у вас есть контейнер DI, так что у вас будет слабая связь с этим хранилищем»

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

Заметьте, я не говорю ничего хорошего или плохого, но должен ли инженер изменить это или нет, будет понятно инженеру, чей код проверяется. Очевидно, что вы должны закончить проверку кода с помощью yay или nay, но накопление этих утверждений в течение всего этого смягчит Nay, поскольку объяснение уже было сделано в форме причинно-следственных операторов, когда вы говорите им: «Я бы хотел эти магические числа исправлены перед проверкой ".

Джимми Хоффа
источник
4
+1 за «простую передачу понимания - это похвала другому инженеру ... это дает форму неявной проверки»
Рой Тинкер
Я хочу +1 это дважды. Один по той же причине, что и @RoyTinker, а другой - «Не говори что-то хорошее или плохое, а скорее говори через причину и следствие». Оба очень хорошие моменты!
Бен Ли
7

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

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

FrustratedWithFormsDesigner
источник
6

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

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

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

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

tylerl
источник
4

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

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

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

Эрик Дитрих
источник
Это напоминает мне концепцию, называемую «Оценочный запрос», которая направлена ​​на то, чтобы улучшить и расширить то, что уже работает, а не на решение проблем.
3

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

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

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

JeffO
источник
2

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

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

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

Фразы, которые я выучил больше всего за эти годы:

  • «Это интересный подход. Что произойдет, если нам придется учесть [какой-то другой вариант использования]?»
  • «Хорошая попытка! Знаете ли вы, что у нас уже есть метод для этого? Может быть, мы должны сделать несколько сравнительных тестов, чтобы увидеть, какой подход является более эффективным».
Джейсон М. Бэтчелор
источник
2

Рабочий процесс, который мне больше всего понравился в обзорах кода, был таким:

  1. Dev представляет патч в список рассылки / онлайн-инструмент.
  2. Каждый, кому нужно заботиться, смотрит на патч, предлагает улучшения.
  3. Dev возвращается к # 1
  4. Если улучшения не нужны, люди говорят: «Хорошая работа, пожалуйста, позаботьтесь». <- ПОЗИТИВНАЯ ОБРАТНАЯ СВЯЗЬ. Весь принятый код хорош. Чем меньше людей говорят вам что-то менять, тем лучше вы делаете.
  5. Dev фиксирует, переходит к следующему пункту.

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

Преимущества этого подхода:

  1. Все знают, что делают все. Там нет знания монополизации или тайн совершает.
  2. Каждый учится на чужой обратной связи. Это важно. Если обратная связь происходит только между двумя людьми в личной беседе во время сопряжения, то кто-то на другой стороне комнаты не получает такой же выгоды, как если бы это произошло в списке рассылки.
  3. Другие разработчики обычно могут обнаружить некоторые ошибки до того, как они попадут в систему контроля версий.
MrFox
источник
Так что, в основном, вы надеетесь не получить никакой обратной связи вообще. Зачем ходить на работу? Вы можете быть невидимым дома.
1

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

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

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

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

Оставьте бахрому в пабе.

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

Это имело значение для меня. Я не хочу пуховых комментариев или позитива ради позитива. Если весь код, который я написал, дрянной, вы скажете мне, почему, и давайте исправим его и изучим. Но если я что-то делаю правильно, приятно это услышать один раз и некоторое время. Мне не нужно положительное подкрепление для всего, что я сделал, что было «правильно», но даже если это «давайте улучшим X, Y и Z, но все остальное выглядит хорошо», это важно.

peacedog
источник
0

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

aserwin
источник
Мой опыт показывает, что программисты, которые стараются, являются «хорошими парнями» и довольны поддержкой команды, как правило, пишут программное обеспечение, которое работает.
c_maker
Цыпленок и яйцо, наверное. Но вопрос был о пересмотре кода ... который я просто не думаю, что
пришло время поразить
Проверка кода - не время, чтобы определить, работают ли видимые пользователем части программного обеспечения в соответствии со спецификацией.
CVn
0

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

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

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

user619818
источник
Может быть, поэтому большинство программистов - мужчины.
-1

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

Akula
источник
-3

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

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

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

РЕДАКТИРОВАТЬ: Хотя я скажу, что, если что-то впечатляет вас достаточно, ничто не мешает вам выразить вашу похвалу лично. Трекер, однако, кажется, только для обзора производственного кода.

KBKarma
источник
6
«Таким образом, в этой ситуации только отрицательные ошибки имеют какую-либо ценность». - Я не могу с этим согласиться. Если кто-то придумает отличный способ исправить ошибку / реализовать функцию, это абсолютно бесполезно. И важно поддерживать мотивацию. Если вы выделите только неудачу, у вас будут проблемы.
Мат
Плохой выбор слова с моей стороны. В то время как хорошие вещи не бесполезны (написав достаточно ошарашенного в моем времени), ошибки, скорее всего, были созданы для трекера. ОП может, если он того пожелает, оставлять положительные комментарии. Но лично я оставил бы это для разговоров лицом к лицу, поскольку это предотвращает засорение трекера. Кроме того, я очень раздражен, я не могу голосовать за комментарии. :)
KBKarma
1
@KBKarma - если вы чувствуете, что ваш первоначальный ответ не был сформулирован так, как мог бы, пожалуйста, вернитесь и отредактируйте свой ответ, чтобы должным образом отразить ваши мысли. Ищите кнопку редактирования под вашим ответом.