Как найти положительные моменты в обзоре кода?

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

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

Мы также были выбраны в качестве «Технического руководства». Это означает, что мы отвечаем за качество кода, но у нас нет полномочий для внедрения изменений в процессе, переназначения разработчиков или сдерживания проектов.

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

Наш менеджер - MBA, который в основном занимается созданием графика предстоящих проектов. Пока он пытается, он почти не понимает, что делает наше программное обеспечение с точки зрения бизнеса, и пытается понять даже самые основные требования клиентов без объяснения от разработчика.

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

Обзор кода привел к некоторой напряженности в нашей команде. Особенно некоторые из более старых членов задают вопрос об изменениях (I.e. «Мы всегда это делали так» или «Почему метод должен иметь разумное имя, я знаю, что он делает?»).

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

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

Я не думаю, что мои стандарты слишком высоки.

Мой контрольный список на данный момент:

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

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

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

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

Пример проверки кода

  

Эй, Джо,

     

У меня есть некоторые вопросы о ваших изменениях в библиотеке \ ACME \ ExtractOrderMail Class.

     

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

     

... (Некоторые другие моменты, которые не работают)

     

Незначительные моменты:

     
  • Почему «GetErrorMailBody» принимает исключение как параметр? Я что-то пропустил? Вы не выбрасываете исключение, вы просто передаете его и называете «ToString». Почему это?
  •   
  • SaveAndSend Не является хорошим именем для метода. Этот метод отправляет сообщения об ошибках, если обработка почты пошла не так. Не могли бы вы переименовать его в «SendErrorMail» или что-то подобное?
  •   
  • Пожалуйста, не просто комментируйте старый код, удалите его прямо. Мы все еще имеем это в подрывной деятельности.
  •   
174 голоса | спросил RobMMurdock 17 +03002016-10-17T20:50:36+03:00312016bEurope/MoscowMon, 17 Oct 2016 20:50:36 +0300 2016, 20:50:36

19 ответов


117
  

Как найти положительные моменты в обзоре кода?

     

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

Отлично, у вас есть реальная возможность создать ценность для вашей фирмы.

  

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

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

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

  

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

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

Критиковать код, а не автора

Вы приводите пример:

  

У меня есть некоторые вопросы о ваших изменениях в

Избегайте использования слов «вы» и «ваш», скажем, «изменения».

  

Я что-то пропустил? [...] Почему это?

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

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

Поднимите планку, положив обратную связь

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

  

Как найти положительные моменты в обзоре кода?

является хорошим и заслуживает внимания.

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

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

Рекомендации по выбору языка

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

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

Общие рекомендации

Вы могли бы найти точки, чтобы похвалить общие принципы кодирования, под различными парадигмами. Например, у них есть хорошие unittests? Утилиты покрывают большую часть кода?

Ищите:

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

Функциональное программирование

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

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

Объектно-ориентированное программирование (ООП)

Если язык поддерживает ООП, вы можете похвалить соответствующее использование этих функций:

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

под ООП, существуют также SOLID (возможно, некоторая избыточность функций OOP):

  • единая ответственность - у каждого объекта есть один участник /владелец.
  • открыть /закрыть - не изменять интерфейс установленных объектов
  • Подстановка Liskov - подклассы могут быть заменены на экземпляры родителей
  • сегрегация интерфейса - интерфейсы, предоставляемые композицией, возможно, mixins
  • инверсия зависимостей - определены интерфейсы - полиморфизм ...

Программирование Unix принципы :

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

В целом, эти принципы могут применяться во многих парадигмах.

Ваши критерии

Это слишком тривиально - я бы чувствовал себя снисходительным, если бы похвалил за это:

  • Код будет скомпилирован.
  • Существует по крайней мере один способ работы кода.
  • Код будет работать с большинством обычных случаев.

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

  • Код будет работать с большинством случаев.
  • Код будет вызывать разумное исключение, если вставленные данные недействительны.

Правила записи кода для прохождения обзора кода?

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

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

Заключение

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

ответил Aaron Hall 17 +03002016-10-17T22:14:10+03:00312016bEurope/MoscowMon, 17 Oct 2016 22:14:10 +0300 2016, 22:14:10
102

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

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

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

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

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

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

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

ответил plast1k 17 +03002016-10-17T22:11:32+03:00312016bEurope/MoscowMon, 17 Oct 2016 22:11:32 +0300 2016, 22:11:32
92

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

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

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

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

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

ответил Ewan 17 +03002016-10-17T22:29:38+03:00312016bEurope/MoscowMon, 17 Oct 2016 22:29:38 +0300 2016, 22:29:38
24

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

На мой взгляд, наилучшей практикой является всегда фокусироваться на коде и никогда не на автора.

Это обзор кода , а не разработчик , поэтому:

  • «Эта петля никогда не закончится», а не «Ваша петля никогда не закончится»
  • «Для сценария X необходим тестовый пример, а не« Вы не пишете тест, чтобы покрыть этот сценарий »

Также очень важно применить одно и то же правило, говоря о просмотре с другими:

  • «Энн, что ты думаешь об этом коде?», а не «Энн, что ты думаешь о коде Йона?»

Обзор кода - это не время для обзора производительности - это нужно делать отдельно.

ответил tymtam 18 +03002016-10-18T03:39:09+03:00312016bEurope/MoscowTue, 18 Oct 2016 03:39:09 +0300 2016, 03:39:09
13

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

Почему вы просматриваете весь запрос слияния в одном сообщении?

Мой опыт с обзорами кода через GitLab; Я всегда думал, что другие инструменты проверки кода будут работать аналогичным образом.

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

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

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

  

Хороший подход, обертывающий это в специальной функции!

Или это может сказать,

  

Это имя объекта не соответствует цели объекта; возможно, мы могли бы использовать такое имя, как «XYZ»?

Или, если есть серьезные проблемы с разделом, я могу написать:

  

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

     

(Пример: Функция ABC на самом деле делает три вещи здесь: bazzing foo, запрещающий boz и отсечение zorf. Все это могут быть отдельные функции.)

После написания всего вышеизложенного я могу сделать сводный комментарий для всего запроса на слияние, например:

  

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


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

  

Извините, но этот код на самом деле не до табака. Существует очень много краевых случаев (как описано в отдельных комментариях), которые будут обрабатываться некорректно и дать плохой пользовательский опыт или даже повреждение данных в одном случае. (См. Комментарий commit commit 438a95fb734.) Даже некоторые нормальные случаи использования приведут к чрезвычайно плохой производительности приложения (особенности, отмеченные в отдельных комментариях к diff для somefile.c).

     

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

     

Я закрываю запрос слияния до полной перезаписи.


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

ответил Wildcard 19 +03002016-10-19T09:59:56+03:00312016bEurope/MoscowWed, 19 Oct 2016 09:59:56 +0300 2016, 09:59:56
9

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

Просто нет причин, по которым вам следует обращаться непосредственно к Джо. То, что Джо исправляет свои неудачи, - это не твое дело. Это кто-то , это ваш бизнес. Ваша проблема - качество кода. Таким образом, вместо того, чтобы писать запросы /заказы /требования (что, если бы я был Джо, я мог бы просто отказаться, потому что вы не являетесь законным для этого), оставайтесь на своей роли и пишите простой анонимный список todo.

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

Вот пример переформулировки с содержанием, взятым из вашего обзора:

  • Прежде чем мы вносим изменения в библиотеку \ ACME \ ExtractOrderMail Class, нам нужно исправить несколько проблем.
  • Если я что-то пропустил, «TempFilesToDelete» не должен быть статичным.
  • В будущем мы можем вызывать функцию более одного раза за запуск, поэтому нам нужно (что нужно сделать здесь).
  • Мне нужно понять, почему «GetErrorMailBody» принимает исключение как параметр. (и, я буду пограничным здесь, потому что к настоящему времени у вас уже должно быть заключение )
  • SaveAndSend следует переименовать, чтобы лучше соответствовать его поведению, например, для «SendErrorMail»
  • Прокомментированный код следует удалить для цели удобочитаемости. Мы используем subversion для возможных откатов.

Если вы сформулируете такой отзыв, независимо от того, насколько читатель вас ненавидит лично, все, что он видит здесь, - это заметки об улучшениях, которые кто-то должен продолжить позже. Кто ? Когда ? Никто не заботится. Какие ? Зачем ? О том, что вы должны сказать.

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

ответил Arthur Havlicek 19 +03002016-10-19T00:32:44+03:00312016bEurope/MoscowWed, 19 Oct 2016 00:32:44 +0300 2016, 00:32:44
8

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

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

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

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

ответил Eiko 17 +03002016-10-17T21:56:29+03:00312016bEurope/MoscowMon, 17 Oct 2016 21:56:29 +0300 2016, 21:56:29
6

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

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

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

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

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

ответил JeffO 18 +03002016-10-18T00:34:56+03:00312016bEurope/MoscowTue, 18 Oct 2016 00:34:56 +0300 2016, 00:34:56
4

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

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

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

  1. Что бы вы изменили в отношении этого кода, если у вас было время?
  2. Как бы вы улучшили эту область кодовой базы?

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

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

ответил lunchmeat317 17 +03002016-10-17T23:42:21+03:00312016bEurope/MoscowMon, 17 Oct 2016 23:42:21 +0300 2016, 23:42:21
4

Качество без натяжения

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

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

Ваши организации спускаются вниз

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

Зачем спрашивать нас, что вам нужно сделать, чтобы ваша команда была счастлива? Рассматривали ли вы вопрос о своей команде?

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

Построение культуры качества

Культуру качества нужно выращивать, чтобы расти снизу вверх.

Пусть ваш босс знает, что вас беспокоит качество, которое не улучшается достаточно быстро, и вы хотите попробовать применить некоторые советы от The Harvard Business Review .

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

Скажите что-нибудь вроде: «Как вы знаете, [коллега], и мне было поручено обеспечить качество кода, чтобы предотвратить проблемы с производством, которые мы недавно пережили. Я лично не думаю, что я решил эту проблему. Я думаю, что моя самая большая ошибка заключалась не во всех вас в начале. [coorker], и я по-прежнему подотчетен для управления качеством кода, но в будущем мы все вместе стараемся вместе. »

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

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

Совместный обзор кода

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

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

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

Качество - это путешествие

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

Но если это не работает ...

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

ответил Tim Grant 18 +03002016-10-18T05:38:48+03:00312016bEurope/MoscowTue, 18 Oct 2016 05:38:48 +0300 2016, 05:38:48
4

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

  

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

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

Точно так же «я бы сделал это по-другому ...» также конфронтационно, потому что непосредственная мысль разработчика будет « Хорошо, я сделал это таким образом ... У вас возникла проблема с этим? "И снова, это больше конфронтационных, чем должно быть, и отводит обсуждение от улучшения кода.

Пример:

Вместо того, чтобы спрашивать: «Почему вы решили не использовать постоянную переменную для этого значения?», просто укажите «Это твердое кодированное значение должно быть заменено константой XYZ в файле Constants.h "Задавая вопрос, разработчик активно выбрал not для использования уже определенной константы, но вполне возможно, что они даже не знали, что он существует. С достаточно большой базой кода будет много вещей, которые каждый разработчик не знает. Это просто хорошая возможность обучения для этого разработчика, чтобы он познакомился с константами проекта.

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

Хорошо:

  • «Имя этой переменной необходимо изменить в соответствии с нашим стандартом кодирования».

  • «Значение« b »в этом имени функции должно быть капитализировано, чтобы быть PascalCase».

  • "Код этой функции не имеет отступов должным образом."

  • "Этот код является дубликатом кода, найденного в ABC :: XYZ () , и должен использовать эту функцию вместо этого."

  • «Вы должны использовать try-with-resources , чтобы гарантировать, что в этой функции вещи будут закрыты должным образом, даже если возникают ошибки ». [Я только добавил ссылку здесь, чтобы пользователи, не являющиеся java, знали, что означает средства try-with-resources]

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

Плохо:

  • «Я думаю , вы можете улучшить этот код, изменив имя переменной в соответствии со стандартом«

  • " Возможно, было бы лучше использовать try-with-resource для правильного закрытия объектов в этой функции"

  • « может желать еще раз взглянуть на все условия в этой функции. Его сложность N-пути превосходит максимально допустимую сложность нашего стандарта».

  • «Почему здесь отступы здесь 2 пробела вместо нашего стандарта?»

  • «Почему вы написали функцию, которая разрушает наш стандарт сложности n-пути?»

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

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

ответил Shaz 18 +03002016-10-18T18:12:07+03:00312016bEurope/MoscowTue, 18 Oct 2016 18:12:07 +0300 2016, 18:12:07
4

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

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

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

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

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

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

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

ответил David W 18 +03002016-10-18T19:06:59+03:00312016bEurope/MoscowTue, 18 Oct 2016 19:06:59 +0300 2016, 19:06:59
3

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

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

«Почему метод должен иметь разумное имя, я знаю, что он делает?» это то, что я нахожу особенно тревожным. Он знает, что он делает, или, по крайней мере, так говорит, но я этого не делаю. Любой метод должен иметь не только разумное имя, но и иметь имя, которое сразу же очистит читателя от кода, что он делает. Возможно, вы захотите перейти на веб-сайт Apple и посмотреть видео WWDC об изменениях от Swift 2 до Swift 3 - огромное количество изменений, сделанных только для того, чтобы сделать все более читаемым. Возможно, такое видео может убедить ваших разработчиков, что разработчики, которые намного умнее, находят интуитивные имена методов очень важными.

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

ответил gnasher729 17 +03002016-10-17T22:33:35+03:00312016bEurope/MoscowMon, 17 Oct 2016 22:33:35 +0300 2016, 22:33:35
2

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

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

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

ответил Karl Bielefeldt 18 +03002016-10-18T21:04:54+03:00312016bEurope/MoscowTue, 18 Oct 2016 21:04:54 +0300 2016, 21:04:54
2

Я видел это из первых рук и хочу предупредить вас от ответил navigator_ 19 +03002016-10-19T03:24:34+03:00312016bEurope/MoscowWed, 19 Oct 2016 03:24:34 +0300 2016, 03:24:34

1

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

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

ответил HelloGoodbye 18 +03002016-10-18T15:06:23+03:00312016bEurope/MoscowTue, 18 Oct 2016 15:06:23 +0300 2016, 15:06:23
1

Помещения ...

Программисты - решающие проблемы. Мы вынуждены начинать отладку при возникновении проблемы или ошибки.

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

Компьютерные сообщения об ошибках редко подвергаются сомнению и никогда не воспринимаются как личное оскорбление.

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

Выводы ...

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

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

Пример отзыва кода отзыва ...

Перепишите приведенный пример, включающий эти методы:

  
  • Тема:

         
    • Библиотека \ ACME \ ExtractOrderMail Class.
    •   
  •   
  • Принципиальная проблема ...

         
    • TempFilesToDelete является статическим      
      • Последующие вызовы GetMails вызывают исключение, так как файлы добавляются к нему, но никогда не удаляются после удаления. Хотя только один звонок   теперь производительность может быть улучшена в будущем с некоторыми   параллелизм.
      •   
      • TempFilesToDelete как переменная экземпляра позволяет использовать несколько объектов параллельно.
      •   
    •   
  •   
  • Вторичные вопросы ...      
    • GetErrorMailBody имеет параметр исключения      
      • Поскольку он не генерирует само исключение и просто передает это ToString, нужно ли это?
      •   
    •   
    • Имя SaveAndSend      
      • Email может или не может использоваться для сообщения об этом в будущем, и этот код содержит общую логику для хранения постоянной копии и   сообщая о любых ошибках. Более общее название позволит такие ожидаемые   изменяется без изменений на зависимые методы. Одна из возможностей   StoreAndReport.
      •   
    •   
    • Комментирование старого кода      
      • Отключение строки, прокомментированной и помеченной OBSOLETE, может быть очень полезно при отладке, но «стена комментариев» также может скрывать ошибки   в соседнем коде. У нас все еще есть это в Subversion. Возможно, просто   комментарий, в котором точно указано, где находится Subversion?
      •   
    •   
  •   
ответил DocSalvager 26 FebruaryEurope/MoscowbSun, 26 Feb 2017 21:48:48 +0300000000pmSun, 26 Feb 2017 21:48:48 +030017 2017, 21:48:48
0

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

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

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

  •   

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

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

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

В идеале план (для того, как вы можете протестировать исправление, или что делать с рефактором и когда), должен обсуждаться до того, как работа будет выполнена, чтобы они не утопили нагрузку в чем-то, прежде чем выяснять, что у вас есть проблема с ним.

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

ответил Useless 18 +03002016-10-18T12:40:55+03:00312016bEurope/MoscowTue, 18 Oct 2016 12:40:55 +0300 2016, 12:40:55
-1

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

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

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

ответил Stig Hemmer 18 +03002016-10-18T13:54:18+03:00312016bEurope/MoscowTue, 18 Oct 2016 13:54:18 +0300 2016, 13:54:18

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132