Должен ли я сказать кому-то, что их совершение вызвало регресс?

115

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

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

Если это хорошая идея, чтобы сказать им, каковы хорошие способы сделать это, не вызывая обиды или не заставляя их защищаться?

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

Скотт
источник
119
Не проверяйте всю команду, когда отправляете ему это письмо.
Quant_dev
26
Конечно, скажите ему, дипломатично или / и с шуткой. В компании, в которой я работаю, у нас есть панель с именем каждого разработчика. Каждый раз, когда кто-то совершает ошибку, связанную с репозиторием (забывает зафиксировать что-либо, забывает пометить тегом, не компилирует и т. Д.), Разработчик получает «+». Когда у него "+++", он должен оплатить завтрак на следующий день. Странно, но с тех пор, как система была введена в действие, стало меньше «обязательных» завтраков :-)
Джалайн
26
@Jalayn - не шутка - это просто раздражает людей
user151019
29
«Допустим, ради аргумента, что ошибка достаточно тонкая, что автоматические тесты сервера CI не могут ее обнаружить». Почему нет? Это то, что у вас нет теста? Если это так, первое, что вы должны сделать, это написать тест (или пару тестов), которые сейчас не пройдены, но пройдут, когда ошибка будет исправлена. Если это невозможно проверить, почему нет?
Томас Оуэнс
18
@ Томас Оуэнс Потому что это не тот вопрос, который я задаю. :-P В идеальном мире в систему не попадало бы никаких ошибок, потому что мы писали бы совершенный код с первого раза, и на случай, если мы этого не сделаем, был бы исчерпывающий набор автоматических тестов. Это, однако, не идеальный мир, поэтому я спрашиваю, что вы должны делать, когда в ваш код попадает ошибка.
Скотт

Ответы:

38

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

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

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

Тогда либо:

Они: Конечно, это справиться ... ситуация, о которой я не знал ...

Или что-то вроде:

Они: Нет, извините, я не помню, выглядит неправильно для меня.

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

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

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

Джастин
источник
«Они: Конечно, это чтобы справиться ... ситуация, о которой я не знал ...» У меня проблема с этим. Если они эффективно задокументировали изменения, то эта ситуация не должна быть тем, о чем вы могли не знать.
Temptar
1
@temptar Достаточно справедливо - замените «не знал» на «еще не подумал» или что бы вы ни предпочли - я хочу сказать, что, хотя вы можете сами в этом разобраться (например, просмотрев документацию), его обычно быстрее просто спросить. Также много кода не так хорошо документировано, как должно быть.
Джастин
170

Будьте настойчивы, не агрессивны. Всегда предпочитайте говорить что-то вроде «этот кусок кода не работает» против «ваш код не работает». Критикуйте код, а не человека, который написал код.

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

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

оборота Сардатрион
источник
107
+1. Личный любимый подход: «Прежде, чем я начну возиться с этим, была ли причина, по которой вы сделали это таким образом?»
фунтовые
67
+1. «Критикуйте код, а не человека, который написал код».
c_maker
11
+1, это очень похоже на то, что мой советник по браку сказал моей жене и мне, когда я испытываю недовольство тем, что делает ваш партнер, избегайте слова ВЫ , это слишком конфронтационно.
maple_shaft
3
+1, но я не думаю, что слово «ты» является конфронтационным. Там должно быть четкое понимание собственности. Лично у меня были люди, постоянно комментирующие код, который нарушал сборку, потому что они не понимали, что именно они вызвали его. Мне нравится подход @ pdr ... это утверждение неконфронтационное, но в нем есть слово "ты".
Тим Редди
3
Похоже, вы снова вводите новую ошибку. Их исправление могло исправить предыдущую проблему, о которой вы не знаете. Почему бы не пойти к ним и спросить, почему они написали код так, как они это сделали. Это могло бы показать, что есть странная странная особенность языка / дизайна / виртуальной машины, которую он покрывал. Идти и показывать им свое эго [«вот как я могу добиться большего» не поможет им]
monksy
70

Да всегда . Как программист, твоя работа - учиться на ошибках.

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

Tom Squires
источник
3
Часть «учиться на ошибках» не всегда верна. Огромное количество ошибок - это, например, отсутствие валидаторов. Это то, что происходит, даже с опытными разработчиками. Вы многому от этого не научитесь. Вот почему нам нужно иметь достойный контроль качества.
Сокол
2
@Falcon Понимание «нам нужно иметь достойный контроль качества» является примером обучения на ошибках. Вы можете продолжить, подумав о том, «почему у нас нет QA / почему наш QA пропустил эту проблему»
MarkJ
2
@Falcon «Это то, что просто случается» <--- только это знание, которое вы получаете от повторяющихся, но тривиальных ошибок. Был ли у вас опыт, когда вы компилируете и что-то не работает, сначала вы проверяете орфографию и удар, в течение 10 секунд ошибка исчезла. Вы знаете, что «это просто так», иногда вы можете отлаживать их за 10 секунд, а не за 10 часов.
Гэптон
@Gapton и MarkJ: это хорошие моменты! Я не думал об этом.
Сокол
«Как программист, твоя работа - учиться на ошибках». -> «Как человек ...» Учиться на своих ошибках не является чем-то конкретным в этой области.
Бурхан Али
23

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

Если это связано с объяснением людям, как не вводить ошибки, сделайте это.

Однажды я работал в команде, где менеджер проекта никогда не говорил конкретному разработчику, что он допустил ошибку: он организовал встречу со всей командой, где он объяснил, что была допущена ошибка и что был определен новый процесс для подавления такого рода ошибки. Таким образом, никто не подвергался стигматизации.

mouviciel
источник
4
+1 за «принять меры, чтобы избежать подобных ошибок в будущем». Это самая важная часть, ИМО.
CVn
1
The constructive way is to find the bug, fix it and take actions to avoid similar bugs to arise in the future.-> Суть вопроса в том, что вы уже исправили ошибку.
Хьюго
1
Да, но будьте осторожны с введением нового процесса. Если вы вводите слишком много процесса и созываете слишком много собраний, это замедляет темпы развития и отравляет моральный дух всей компании. Я видел слишком много магазинов, слишком остро реагирующих на ошибку одного человека. Новый процесс может быть уместным только в том случае, если ошибка свидетельствует о нарушении процесса.
Джейкоб
@jacob - я согласен.
Mouviciel
19

В общем да .

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

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

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

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

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

Калеб
источник
2
Мне нравится "Я не совсем понимаю это, вы можете объяснить мне, как это работает?" подход. Если это преднамеренно (и недавно), то оригинальный программист должен уметь довольно хорошо объяснить, как работает код. Если это ошибка, есть вероятность, что при объяснении того, что делает код, он / она обнаружит ошибку, и в середине объяснения вы услышите «упс». В любом случае, кому-то будет трудно почувствовать, что на него указывают пальцем на возможную ошибку.
CVn
3
+1 за «это похоже на ошибку, но я хотел, чтобы вы ее запустили перед тем, как возиться с вашим кодом».
Рассел Борогове
6

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

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

Д Крюгер
источник
5

Вы получаете отличные ответы здесь.

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

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

Она почти извиняющимся тоном упомянула мне, что, похоже, возникла проблема, и я успею разобраться?

Достаточно часто ошибка была моей, и она это знала. Это умение.

оборота Майк Данлавей
источник
5

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

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

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

Майкл "Опт" Грамм
источник
4

Хорошей тяги по вашему вопросу! Все сказали вам, что делать. Вы должны сказать? ДА! Каждый раз, когда возникает вопрос: «Должен ли я больше общаться?», Ответ почти всегда ДА!

Но добавить что-то другое: ваша предпосылка ошибочна.

Сотрудник сделал коммит, который не сломал CI, но привел вас к обнаружению проблемы.

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

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

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

tardate
источник
2

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

Скажи им, когда это только вы двое.

Имран Омар Бухш
источник
+1 за последнее предложение. Хвалите на публике, критикуйте наедине.
Скотт С. Уилсон
2

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

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

Дунайский моряк
источник
1
Все дело в том, как вы говорите человеку, что они совершили ошибку. Я все время делаю ошибки и буду рад, если кто-то укажет на них, чтобы я мог их улучшить, но если вы подойдете и скажете мне: «Чувак, твой последний коммит полностью нарушил код. Почему ты не можешь быть лучше, проверяя свои ошибки? ?» Я конечно буду обижаться.
Кувшин
Да, однако вопрос "Чувак, ты запускал тесты junit перед коммитом?" это, я думаю, вполне приемлемо :)
Данубский моряк
+1 за Только те, кто ничего не делают, не делают ошибок . Очевидно, когда он сформулирован, но я не видел его так аккуратно, как раньше.
FumbleFingers
2

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

пушистый
источник
2

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

Да, обязательно расскажи им об этом, но не делай это перед всей командой

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

Я обычно нахожу, что это работает лучше всего, когда вы начинаете с комплимента, а затем получаете сообщение об ошибке ... что-то вроде «исправление, которое вы реализовали, работает отлично, НО, кажется, оно сломало x, y, z» или «спасибо за то, что вы , b, c, НО это, кажется, вызывает x, y, z "

Рейчел
источник
2

Простой ответ: да.

Более длинный ответ: моя последняя работа была в Agile компании, которая использовала TDD с инструментами CI, чтобы гарантировать, что то, что было в нашем репозитории SVN, было хорошим, рабочим кодом всегда. Когда что-то было зафиксировано, наш сервер TeamCity получил копию, скомпилировал и запустил модульные тесты. Он также запускал интеграционные тесты ежечасно. Если что-то совершалось, что приводило к сбою CI, каждый получал электронное письмо с сообщением о том, что сборка была нарушена на основе фиксации определенного человека.

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

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

оборота KeithS
источник
В небольшом стартапе, где я работал, у нас была похожая система. Самое смешное, что когда вы зарегистрировали какой-то код и тесты провалились, система сборки возложила вину за провал теста на человека, который в последний раз проверил правку в строке, в которой тест / компиляция провалилась. Так что, если я удалил функцию, которую вы использовали, и ваш код не удалось собрать. Build-Bot будет неистово винить вас. Последовавшее за этим ругательство и дружественный вызов имени гарантировали, что ошибки сборки были исправлены быстро, и раздражение каждого было направлено на Build-Bot.
Стюарт Вудворд
2

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

Стюарт Вудворд
источник
1

Есть много факторов в игре.

  • Насколько серьезна ошибка?
  • Каковы отношения старшинства между вами и нарушителем?
  • Насколько занята / напряжена команда?
  • Работал ли прерыватель в их части кодовой базы или в вашей?
  • Насколько вы уверены, что это была настоящая ошибка, и насколько вы уверены, что ваше исправление верно?

Если проблема была незначительной - ошибка опечатки / Thinko / Cut & Paste - и прерыватель является занятым партнером, и вы уверены в своей оценке проблемы, вам, вероятно, не нужно обращать на это внимание. (например foo.x = bar.x; foo.y = bar.y, foo.z = bar.y).

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

Если природа ошибки указывает на серьезное недопонимание (платформы реализации, локальных политик или спецификации проекта), тем не менее, сообщите об этом как можно скорее.

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

Рассел Борогове
источник
1

Что произойдет, если вы не скажете им?

Минусы

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

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

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

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

Плюсы

Люди не смущаются и не раздражаются из-за того, что указывают на свои ошибки.

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

HLGEM
источник