Проблема в том, что пулл-реквесты одобряются без комментариев?

Короткий ответ - нет

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

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

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

Я также самый старший разработчик в команде

Вот это может быть важно. В этом случае я легко могу представить, что к вам относятся с особым почтением/уважением, и может применяться сочетание следующего (я видел это лично):

• другие члены команды не хотят, чтобы вы видели, что они бросают вызов качеству вашего кода; и/или

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

  и поэтому:

• они просто «штампуют» ваши запросы на вытягивание — что вы и видите.

Это особенно верно, поскольку у вас есть:

недавно присоединился к новой компании

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

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

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

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

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

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

На вашем месте я бы задал ожидания вежливым и скромным толчком, например: «Я собираюсь отправить вам запрос на включение [Функции X]. Я немного не уверен в том, как я исправил [Проблему Y]. Не могли бы вы пожалуйста, взгляните и дайте мне знать, если обнаружите какую-либо проблему? Кроме того, есть ли у вас какие-либо советы о том, как я могу провести рефакторинг [Раздел Z]? Я думаю, что это может быть трудно понять и поддерживать в будущем, но я не вижу элегантное решение».

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

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

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

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

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

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

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

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

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

(И этот вопрос, вероятно, относится к Software Engineering SE.)

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

Если у них нет времени взглянуть на ваш код, они дадут вам знать об этом.

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

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

Вообще говоря, это плохая жизненная философия. Что касается программного обеспечения, мы знаем, что код, который был протестирован и признан достойным производства, всегда содержит ошибки. У меня был профессор в колледже, который утверждал, что в большинстве программ примерно 1 ошибка на 100 строк кода, но я не могу найти источник для этого прямо сейчас. Кроме того, ошибки стоят дорого: некоторые утверждают, что ошибка стоит 10 тысяч долларов, если она обнаружена в продакшене .

Стоит потратить некоторое время на просмотр кода и, по крайней мере, задать вопросы о нем.

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

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

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

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

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

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

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

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

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

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

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

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

Несколько вещей из личного опыта, которые могут помочь с вашей проблемой:

• Держите PR маленькими и простыми. Просмотр небольшого PR, который решает одну четко определенную проблему, намного проще, чем внесение множества различных изменений в несколько файлов. Чем больше PR, тем больше вероятность того, что ваши коллеги просмотрят его и пропустят важные изменения.
• Если есть конкретные изменения, о которых вы хотите получить отзыв, прокомментируйте их и объясните, что вас беспокоит. Это привлечет внимание людей к изменению и побудит их поделиться своим мнением.
• Отметьте больше людей в своих PR и потребуйте как минимум 2 или 3 одобрения перед слиянием.

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

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

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

Здесь много хороших ответов, но я не заметил никого, кто предложил бы это в вашем PR:

• Отметьте, что вы исправили
• Упомяните, как вы это тестировали
• Попросите их посмотреть на/на конкретные вещи

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

Это означает снять шляпу «Я написал это» и надеть шляпу «Это написал кто-то другой».

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

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

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

Поскольку это Workplace.SE, а не Software Engineering SE, я отвечу на этот вопрос с общей точки зрения «найма новых инженеров».

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

У нас в компании появился новый сотрудник (инженер, а не программист). Сначала ему поручили только один проект — один из моих проектов — и он был явно разочарован тем, что не получил должного контроля. Но у меня были срочные дела (поскольку у нас не хватало рук в течение нескольких месяцев), и я мог помочь ему только с теми вещами, которые нужно было сделать совершенно прямо СЕЙЧАС (т. е. с теми, которые выходили за пределы компании к клиенту и поставщикам). Он активно взял на себя подготовку списка материалов для внутреннего использования компанией для проекта (не в форме компании, поскольку ему не показали, где она находится). Его формат оказался значительно лучше формы компании, поэтому Я сказал ему придерживаться этого, но внести некоторые коррективы в содержание. Теперь у него есть несколько проектов, над которыми нужно работать, так что проблема решена - на самом деле всего пару недель спустя,

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