Коллега просматривает код слишком поздно в процессе

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

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

Отзывы варьировались от несколько полезных до несколько бесполезных. Мы все равно объединились, Masterи на следующий день я спросил, мог ли он проверить этот код раньше, вместо того, чтобы ждать завершения интеграционного тестирования. Когда он оставил отзыв, это был конец дня, и мы готовили заявку для нашего релиз-инженера для развертывания в 4:30 утра следующего дня.

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

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

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

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

РЕДАКТИРОВАТЬ : мне неловко, что я упустил эту деталь, но наши инженеры открывают запросы на вытягивание из своих веток функций development, куда должна идти эта обратная связь (по этим запросам на вытягивание)... когда все эти изменения готовы для перехода в нашу производственную среду ( после того, как наши QA-инженеры проведут интеграционное тестирование и удостоверятся, что изменения, по их мнению, безопасны) мы открываем PR для слияния Masterпосле того, как инженер одобрит изменения и согласится, что мы не вносили ничего ужасного, а затем развертываем на следующее утро.

Какова роль Клинта в процессе PR/слияния? Вам нужно, чтобы его обзор был разрешен для слияния? Он просто дает советы?
Каков общий процесс здесь? Из того, что вы написали, было проведено интеграционное тестирование, и у вас есть PR, открытый для слияния с мастером, и вы также подразумеваете, что в результате выполняете развертывание с мастера. Почему PR делается после интеграционного тестирования? Как долго был открыт PR? Какие еще возможности существовали для обратной связи? Что произойдет с вашим тестированием, если будет сделан комментарий по связям с общественностью, который выдвигает на первый план фундаментальную проблему? Похоже, что здесь есть что обсудить в более широком контексте со всей командой, и это не обязательно проблема с тем, что Клинт оставляет комментарии к PR, имхо.
Почему вы ждали до последнего возможного момента, чтобы слиться? Есть ли причина, по которой вы не стремились объединиться несколькими часами ранее, и в этот момент, вероятно, было бы достаточно времени, чтобы ответить на любые комментарии? Если запрос на вытягивание открыт, ожидается, что люди будут оставлять комментарии — это больше похоже на проблему на вашей стороне, чем на их стороне. Кроме того, откуда он должен был знать, что не должен комментировать? Обратите внимание: если сами комментарии требуют, чтобы вы в основном тратили время на изменение вещей без необходимости, у вас есть другой вопрос.
Это вопрос для OP, чтобы уточнить, но для всех комментаторов, предполагающих, что PR должен был быть сделан раньше, если их процесс похож на то, где я работаю (поток git), то PR / слияние с мастером - это то, как развертывание в производство сработал. Было бы много более ранних PR для ветки разработки (или магистрали) для каждой отдельной функции и PR для различных веток выпуска для QA или других сред. Проверка кода должна была произойти тогда, а не во время финального слияния с мастером.
@sleske Отличный вопрос! Это может быть источником нашей проблемы, но я вижу его роль в том, чтобы сообщить нам, что никакие из наших изменений не являются вредными, основываясь на том, что он понимает о платформе бронирования. Я могу слить без его обзора, а он просто дает советы
@Moo Спасибо за вдумчивый ответ, я надеюсь, что смогу достаточно хорошо описать весь процесс: инженеры по контролю качества объединяют ветки функций одну за другой в нашу ветку разработки, затем мы развертываем ветку разработки в нашей среде разработки для интеграционного тестирования. Если тесты пройдены, они объединяют разработку с мастером дымовых тестов в промежуточной среде. Изменения объединяются в мастер только после прохождения интеграционных тестов и посредством запроса на включение в качестве дополнительной возможности отлова критических изменений. Я хотел бы, чтобы его отзыв был раньше, когда мы сможем правильно отреагировать, но он не хочет его давать тогда.
@Dukeling Мы стараемся синхронизировать ветку Master с производственной и развертываем ее вскоре после слияния с ней. Наши QA-инженеры ожидают один день для тестирования изменений в ветке разработки, прежде чем объединиться с Master для дымового тестирования и развертывания в рабочей среде вскоре после этого. Мой вопрос заключается в том, чтобы ограничивать обратную связь вещами, которые могут заблокировать развертывание, когда человек, предлагающий обратную связь, не хочет такого рода ограничений.
@DavidConrad Да, достаточно близко. Это не происходит автоматически, но мы стараемся держать новые коммиты вне Master до тех пор, пока они не будут тщательно протестированы и мы не готовы к развертыванию в рабочей среде.
Код-ревью его код-ревью? Code Review Review: «Интересная обратная связь. Местами неправильная. Требуется более активный подход. Практикуйте более точное время».
Вы не упомянули, что когда-либо сообщали Клинту, когда вы ожидаете, что он завершит отзыв. Вы вообще сообщали ему о какой-либо временной шкале? Если у вас есть сроки, с которыми ему нужно работать, он должен знать об этом, чтобы сообщать о своих задачах и приоритетах своему начальству.

Ответы (10)

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

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

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

+1 это правильный ответ. Обратная связь есть обратная связь, но, как говорится в ответе, если «Клинт» не обнаружил какую-либо проблему с блокировщиком, обратная связь будет рассмотрена позже. Если «Клинта» это не устраивает, они могут просмотреть код раньше.
Вы бы по-прежнему говорили, что это его проблема, если бы он не знал, что они хотели слиться в тот вечер?
Это не его проблема, если это была его первая реальная возможность оставить отзыв. И даже если бы это было так, его проблема в том, что ваша проблема — это общая проблема, потому что вы все — часть одной команды и все хотите, чтобы она преуспела, не так ли?
Я не понимаю, почему это проблема Клинта. Клинт — часть другой команды, похоже, он дает обратную связь независимо от процесса. Проблема в том, что ОП не считает это очень полезным (из-за времени). Я думаю, что ни у кого нет четкого представления о том, о чем должен быть обзор кода, или у каждого есть свое представление. Он может служить многим целям. Они должны попасть на одну страницу.
Я думаю, что этот ответ правильный - я просто не вижу здесь никакой «проблемы». Может я неправильно понимаю? Клинт предоставил новую информацию, часть которой ОП считает ценной. Потрясающий. Это всегда хорошо. Если эта информация указывает на то, что вы не можете выполнить развертывание или слияние или что-то еще, ЛУЧШЕ узнать об этом как можно скорее. Если эта информация указывает на незначительные проблемы, отлично, зарегистрируйте ее и отсортируйте соответствующим образом. Здесь нет проблем. Если вы хотите более чутко реагировать на отзывы Клинта, то, конечно, вам нужно привлечь его к участию раньше, но я по-прежнему считаю, что это вполне нормальная ситуация как есть.
Вот как компания должна относиться к этому. Ошибочно думать, что каждый комментарий в обзоре кода требует действия. После того, как вы обнаружите какие-либо очевидные ошибки (надеюсь, это сделает первый человек, который сделает обзор), многие комментарии будут касаться «вещей, которые было бы неплохо сделать», или «вещей, которые нужно сделать лучше в следующий раз», или просто «вещей, которые вы могли бы сделать по-другому, но может быть, продолжит делать то же самое».
Лол, это проблема Клинта в том смысле, что ЕСЛИ он хочет, чтобы его отзыв воспринимался серьезно, ТО Клинту нужно предоставить такой отзыв на достаточно раннем этапе процесса.
Комментарий на конец дня для развертывания в 4:30 утра следующего дня? Черт возьми, я изменил процедуры, чтобы разрешить развертывание только с понедельника по среду, потому что кто-то ошибся в пятничном развертывании после рабочего дня и создал огромную гору работы в понедельник. Клинт не должен был ожидать, что отзывы будут действенными, за исключением самых серьезных ошибок типа «шоу-стоппер».
Это не касается того, как обсудить этот вопрос с командой. Но это довольно очевидно — вы объясняете команде, что это очень полезная обратная связь, которая позволит им работать лучше в следующий раз. Ни один развернутый код никогда не бывает идеальным, и всегда есть вещи, которые вы можете сделать лучше, и тот, кто указывает вам на это, помогает вам учиться.
Я бы также добавил, что это также вопрос управления. Все обзоры кода должны быть выполнены до QA. Исправления кода до QA дешевы, исправления кода после QA дороги, потому что они должны снова пройти QA; это исправляется только в том случае, если это шоу-стоппер. Если кто-то делает обзор кода после QA, это тратит деньги компании. Либо нужно заниматься процессом так, чтобы код-ревью происходил вовремя, либо вообще не заморачиваться.
@РобП. Я вижу проблему в том, что ОП слишком серьезно относится к отзывам Клинта. Это явно не относится к готовящемуся релизу, так как Клинт вряд ли будет идиотом, и поэтому его следует иметь в виду для будущих релизов.

Либо проверка кода требуется для релиза, либо она не требуется. Если это требуется, то рецензент должен нести ответственность за своевременную проверку. У вас должна быть возможность подойти к его столу и сказать: «Бросьте все остальное и сделайте этот обзор, иначе мы не сможем назначить дату релиза». И у вас должна быть возможность сказать: «Извините, мы не смогли выпустить вовремя, потому что проверка не была сделана вовремя».

Если не требуется, то отпускаете.

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

+1 - ответственность без возможности действительно что-то делать - это проблема, с которой я сталкивался чаще, чем хотел бы.
Учитывая, что «Клинту», по-видимому, было дано меньше дня на рассмотрение человеко-недель или, возможно, человеко-месяцев работы, этот ответ кажется довольно далеким от истины. Делать рецензента «ответственным» за проверку кода быстрее, чем это реально возможно для него, явно неразумно. И поскольку Клинту, по-видимому , удалось оставить несколько полезных комментариев за те несколько часов, которые у него были, кажется еще более извращенным предположение, что что-то было бы исправлено, если бы только у ОП была возможность заставить Клинта отказаться от своих собственных приоритетов и пересмотреть или вините Клинта в том, что он слишком медлителен.
@MarkAmery Я не вижу в вопросе никаких указаний на то, что время Клинта было ограничено OP. Если бы у OP была возможность спросить: «Сколько времени вам нужно для проверки?» а затем власть сказать: «ОК, поэтому вам нужно начать в дату X или раньше, потому что нам нужна обратная связь до даты Y», тогда все будет в порядке. Если ОП не может этого сделать, он не должен нести ответственность за отсутствие ответа на отзыв. Это должна быть проблема управления, а не ОП. Речь идет не об обвинении Клинта, а о том, чтобы не требовать от ОП и его команды невозможного.
+1 Кроме того, команда OP может ответить на отзывы, не связанные с блокировкой, до следующего выпуска.
@Mołot Дядя Бен Питера Паркера был близок, когда сказал, что «с большой силой приходит большая ответственность». Они не собираются вместе. Это одно и то же. Если у меня нет власти контролировать, как что-то делается, я не несу ответственности за то, что это сделано неправильно. Слишком много людей пытаются отделить власть и ответственность друг от друга, но это просто невозможно, и попытки сделать это терпят неудачу. Всякий раз, когда кто-то пытается возложить на меня ответственность там, где у меня нет власти, я говорю: «Как вы думаете, что именно, по вашему мнению, я мог сделать по-другому, чтобы предотвратить ${BAD_THING}?»; ответ обычно сводится к «ничего».
@MontyHarder Переопределение термина «ответственный» в соответствии с текущим обсуждением вряд ли принесет пользу. Если они спрашивают вас о рецензии и ожидают вашего ответа , а вы отвечаете (главным образом потому, что внутренне привязываете ответ к своему профессиональному представлению о себе), то это все: вы несете за это ответственность настолько, насколько это возможно. Вы можете соответствовать такому определению, не обладая полной властью или контролем над каждым аспектом.
@Mołot Мое прочтение последовательности событий заключалось в том, что между открытием PR (и, следовательно, доступным для просмотра Клинтом) и его объединением, несмотря на его возражения, прошло менее 24 часов. Эта интерпретация теперь подтверждается ФП. Он был открыт в какой-то момент рабочего дня, Клинту удалось втиснуть какой-то обзор, а затем команда проигнорировала все это и объединилась в 4:30 утра следующего дня, когда ОП обвинил Клинта в том, что он ранее не рассмотрел PR, который не еще не существует . Мне это кажется довольно сумасшедшим.
@MarkAmery в таких обстоятельствах проблема с расписанием. И я все еще думаю, что ОП должен либо иметь возможность спросить, сколько времени нужно Клинту, и соответственно планировать, либо, если у него нет такой силы, ОП не следует обвинять в том, что у Клинта было мало времени, или обзор фактов был проигнорирован. Виноват только тот, кто разработал такой слишком плотный график.
@ Mołot Конечно, это может быть не в руках ОП. (Или не может. Я думаю, вы предполагаете, что расписание находилось вне контроля ОП, но ничто не подтверждает это в вопросе; ОП управляет большой командой и правдоподобно устанавливает или, по крайней мере, влияет на их расписание.) Несмотря на это, это не искупает ответ, который обвиняет ситуацию конкретно и исключительно в отсутствии у ОП власти над Клинтом , когда ОП признает, что Клинт немедленно оставил полезный отзыв в тот же день, когда ему дали что-то для проверки. Никакая сила не позволит OP заставить Клинта просмотреть код до его появления.
@MarkAmery Согласно вопросу, Клинта пригласили на проверку кода, но он не согласился. Нет никаких доказательств того, что OP удерживает код до конца процесса. Клинт, по-видимому, сам решает просмотреть только в конце процесса. Вывод таков: Клинт не думает, что то, что он может получить, не стоит его времени.
@DavidThornley Да, согласно вопросу, Клинта теперь приглашают на собрания по проверке кода. Даже если мы примем его отказ от них как доказательство того, что он решил не проверять до конца процесса (в отличие, скажем, от того, что они совпадают с более важными встречами, или от того, что его менеджер дал ему указание не тратить время на работу другой команды). для них), это не имеет ничего общего с предыдущим инцидентом, описанным ОП. Вывод вообще не следует, так как у нас нет информации о том, почему Клинт отказался от встреч.

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

Проблемы с процессом:

  • Какова цель этого запроса на включение? Предназначено ли это для проверки или это просто средство для слияния с мастером из проверенной и протестированной ветки? Если первое, то вам следует подумать о другом графике процесса проверки. Если последнее, оно должно быть объединено практически в момент его создания.
  • Что вы делаете с «поздними» отзывами? Похоже, что комментарии Клинта не прошли полную проверку, хотя и были «поздними». (Что означает «опоздание»?) Даже если вы взяли на себя обязательство объединиться и ни один из комментариев не мешает шоу, должна быть возможность классифицировать его комментарии как требующие действий или нет, и ответить в PR соответствующим образом.

Личные проблемы:

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

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

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

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

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

Чтобы быть более точным в отношении «занятости», Клинт может чувствовать, что его втягивают в команду и рабочую нагрузку, которая не входит в его обязанности, когда у него уже достаточно (возможно, даже слишком много!) собственных забот. В этом случае включение его в отчеты об ошибках и другие усилия, чтобы заставить его чувствовать себя более вовлеченным/ценным, могут на самом деле работать против отношений.
Инженеры поддержки IME часто перегружены. Было бы полезно выяснить причины, по которым Клинт отклоняет приглашения на проверку кода (возможно, он просто слишком занят). Сообщить Клинту, что его вклад ценен, пусть и поздно, было бы хорошим способом приблизить его к команде OP. Если у Клинта есть способности, но он чувствует, что он недостаточно близок, чтобы вмешиваться раньше, он может просто чувствовать себя некомфортно из-за того, что наступает на пятки другим людям.
Да, именно поэтому ты идешь пить кофе и обсуждать это. На этот раз он, похоже, вызвался добровольцем, так что проверить, хочет ли он внести свой вклад и каким образом, это кажется самым прямым способом урегулировать ситуацию, а в неформальной обстановке он может выпустить пар, если захочет. не ценился.

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

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

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

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

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

4) Что касается отсутствия Клинта на «совещаниях по обзору кода»: во-первых, я не уверен, что такое «совещание по обзору кода». Проверка кода — это асинхронные операции: разработчик отправляет код, рецензент кода выполняет проверку, пока разработчик делает что-то еще, проверка завершается, разработчик обрабатывает комментарии, полоскание, повторение. Я не знаю, что такое «совещание по обзору кода», это звучит глупо. Но помимо этого, если Клинт в вашей команде, Клинт должен присутствовать на собраниях команды. Если Клинта нет в вашей команде, Клинту не нужно посещать собрания команды. Это просто.

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

Кто-то должен определить «процесс», например, последовательность, в которой что-то происходит.

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

Я не понимаю часть вопроса, в котором говорится: «Отклонил все наши приглашения на встречи по проверке кода в этом месяце».

Кстати, я не уверен, что такое «совещание по обзору кода». Моя идея проверки кода такова:

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

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

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

Можете ли вы сделать «интеграционное тестирование» проще, возможно, более автоматизированным? Чтобы вы могли сделать:

  • Разработка
  • Интеграционное тестирование
  • Рассмотрение
  • Исправить комментарии к обзору
  • Снова интеграционное тестирование

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

-1 за «обзор кода должен быть сделан после тестирования». Некоторое тестирование может быть автоматизировано, но большая часть тестирования выполняется вручную (это особенно верно для внешнего кода, но может быть верно и для внутреннего кода). Ручное тестирование может занять много времени; обзоры кода, как правило, короткие.
Под «после тестирования» я подразумеваю, как минимум, после повторного запуска автоматических дымовых тестов плюс все, что вы делаете, чтобы проверить, что новая функция работает, как указано. Различные системы имеют разную степень автоматизации при тестировании (и я сказал, что если бы их тестирование было более автоматизированным, они могли бы позволить себе делать это чаще).
Тестирование во время разработки отличается от тестирования при подготовке к выпуску, и в зависимости от тестов они могут занимать значительное количество времени, поэтому вам нужно ограничить частоту их запуска.
@JoeW Да, и «ночь перед большим релизом» звучит довольно [слишком] поздно. Я предполагаю, что «отзывы варьировались от несколько полезных до несколько бесполезных» означает, что он не нашел никаких препятствий для шоу, и поэтому ОП был прав, выпустив вовремя, когда они это сделали. И это не так уж плохо — тот факт, что проверка кода не находит недочетов, не обязательно является плохим. В чем заключается его работа, как не в том, чтобы «учить моих инженеров»? Просто посмотреть (т.е. покурить) изменения в ветке интеграции перед релизом?
@ChrisW Я определенно согласен с тем, что непроверенный код не следует проверять (я бы сделал еще один шаг и сказал, что его вообще не следует коммитить!). Однако, как сказал Джо, тесты, которые разработчик должен проводить для своего кода, отличаются от тестов, которые будет выполнять QA. Бывший, точно. Последнее не очень.
Я думаю, что вы пропустили критическую проблему при любом тестировании до того, как просмотрели сам код. Если интеграционные/дымовые тесты будут выполняться в течение значительного времени, имеет ли смысл запускать тесты, а затем проводить экспертную оценку, которая может выявить проблемы, требующие повторного запуска тестов? Не было бы более разумно провести проверку заранее, чтобы можно было обнаружить любые потенциальные проблемы до того, как начнется тестирование, чтобы вы свели к минимуму частоту проведения этих тестов?
@JoeW Я полагаю, вы правы, т. Е. ОП подразумевал, что его «интеграционные тесты» были дорогостоящими или трудоемкими - так что, может быть, в «процессе» следует указать, что проверка должна проводиться перед интеграционным тестированием? Если до интеграционного тестирования, то до или после интеграции (я не знаю)? Возможно, я пытался оправдать Клайва, т.е. угадать, каковы могли быть мотивы Клайва. Если он хочет проверить это поздно, то, возможно, его проверку следует рассматривать как часть интеграционного теста, то есть просто «пройти / не пропустить», а все его полуполезные комментарии в стиле отбросить ...
... или сортировать, например, ОП может по желанию добавить их в рекомендации по проверке кода своей команды или использовать их для обучения проверке кода.
«Проверка кода, который не работает, называется «разработкой и отладкой» и требует больше времени». Я думаю, что это скорее компромисс, и это может зависеть от способностей/опыта разработчиков. Проверка перед тестированием QA может сэкономить в целом, если вы обнаружите серьезные проблемы, которые означают в значительной степени переделку работы; нет большого смысла в тестировании кода, который нужно в значительной степени выбросить (например, вы обнаружите, что какой-то код, извлекающий несколько тысяч результатов отчета, захватывает связанные данные по одной записи за раз). Я стараюсь не искать настоящие ошибки, а вместо этого сосредотачиваюсь на таких вещах, как общий подход и нормы.

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

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

Остерегаться.

✓ Старший разработчик
✓ Находит проблемы в коде, который никто не
замечал ✓ Команда обескуражена из-за позднего отчета
✓ Сообщает что-то «не о своей работе», но все равно проверено
✗ Готовится к повторению того же сценария

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

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

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

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

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

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

«Проблема в том, что ваш рабочий процесс несовершенен». Я не понимаю эту перспективу. Мне кажется, что команда пытается заранее провести рецензирование, и проблема в том, что Клинт игнорирует их до последней минуты. Рабочий процесс кажется нормальным, но Клинт, похоже, отказывается принимать в нем активное участие.
@DarthFennec Зачем вам вообще проводить интеграционное тестирование перед рецензированием кода? Мне это кажется ошибочным, так как вам придется повторить любое из проведенных тестов, если обнаружится какая-либо проблема с кодом в обзоре.
Я не согласен с этим. Я говорю, что ОП, похоже, тоже не согласен. «На следующий день я спросил, мог ли он просмотреть этот код раньше, вместо того, чтобы ждать завершения интеграционного тестирования». Похоже, что OP и его команда пытаются провести экспертную оценку перед интеграционным тестированием, и заявленная проблема заключается в том, что рецензент (Клинт) действует против этого рабочего процесса.
@DarthFennec Частично проблема здесь заключается в отсутствии деталей в исходном вопросе, что позволяет нам читать очень разные основные повествования в истории. Вы читали, я полагаю, что команда хотела, чтобы Клинт проверил, но он уклонялся от своих обязанностей до последней минуты. Я читал, что Клинт не участвовал в проекте и не имел над ним власти, а затем попал в засаду в день дедлайна с гигантским пиаром из человеко-месяцев дерьмового кода, который у него не было возможности проверить до этого, а затем ОП разозлился на «поздний» отзыв и объединил его вопреки возражениям Клинта.
@MarkAmery «затем попал в засаду в день крайнего срока», разве это не противоречит поведению ОП? Почему OP сказал: «Я спросил, мог ли он просмотреть этот код раньше, вместо того, чтобы ждать до завершения интеграционного тестирования», если Клинт попал в засаду с кодом после интеграционного тестирования? Почему ответ Клинта будет «это не моя работа», а не «я не знал об этом до интеграционного тестирования»? Я не говорю, что вы не правы, я просто не понимаю, как вы пришли к такому выводу. Я согласен с тем, что было бы лучше, если бы ОП был более ясным в этом вопросе.
@DarthFennec Я согласен с тем, что позиция ОП кажется иррациональной, учитывая мою интерпретацию, но это подтверждается редактированием ОП. Последовательность событий такова: 1. группа разработчиков, в которых Клинт не играет никакой роли, выполняется командой OP, 2. открывается релизный PR, 3. Клинт вызывается для проверки работоспособности и делает это в тот же день, уезжая. некоторые негативные отзывы, 4. релиз в любом случае происходит в 4:30 утра на следующий день, 5. ОП жалуется Клинту на «поздний» отзыв. Комментарий «это не моя работа» кажется разумным от человека, который вышел за рамки служебного долга, чтобы оставить обширный отзыв для членов ...
@DarthFennec ... другая команда разработчиков, а затем их отругали за это и сказали, что они должны были сделать больше и раньше в проекте, который им изначально не принадлежал. Ответ ОП на это просто кажется мелким перекладыванием вины без особых оснований, что, по крайней мере, является чем-то человечным и понятным.
@MarkAmery Из редактирования: «мы открываем PR для слияния с Мастером после того , как инженер одобрит изменения». Я интерпретировал это как означающее, что Клинт был одним из этих инженеров, и он ждал, чтобы утвердить изменения, пока не был сделан PR для Мастера. , вместо того, чтобы делать это до или параллельно с интеграционным тестированием QA, как он должен был. Но это действительно может пойти по любому пути. Тот факт, что ОП несколько раз спрашивали, какова роль Клинта, и он не ответил прямо на это в редактировании, является подозрительным; если он намеренно уклоняется от ответа на вопрос, это придает правдоподобность интерпретации перекладывания вины.
@MarkAmery Нет ничего, что указывало бы на необходимость участия Клинта. (Действительно, развертывание было выполнено до того, как его комментарии можно было рассмотреть.) OP приглашает Клинта на проверку кода и боится, что комментарии в последнюю минуту, которые не могут быть полезны для этого выпуска, повторятся. ОП явно хочет, чтобы Клинт помог обучить свою команду, а Клинт, очевидно, не хочет этого делать. Любое обвинение в перекладывании вины действительно должно показывать, что вина вообще существует, а я этого не вижу.

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

<Сводка изменений>

Это предварительный обзор. Его цель - найти основные проблемы, такие как:

  • Объединение ошибок
  • Случайное включение неполной функции
  • Впечатляющие ошибки

За исключением подобных проблем, этот запрос на вытягивание будет объединен в <дата и время>.

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

  • <Ссылка на PR 1>
  • <Ссылка на PR 2>
Коллега просматривает код слишком поздно в процессе
СпросиСетьПолитика конфиденциальности1y, 0v