Что делать, если разработчик, чей код я рецензирую, занимает оборонительную позицию?

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

Такова печальная реальность жизни в компании.

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

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

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

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

Еще одна важная вещь, которую вы можете сделать, это объяснить, почему что- то нужно изменить. Не говорите просто: «Эй, поменяй X на Y». Объясните ему, что X вызывает ошибки в реалистичном сценарии Z или что Y чище и упрощает поддержку кода. Снова попытайтесь сформулировать это таким образом, чтобы ваш коллега почувствовал, что вы активно пытаетесь помочь ему стать лучше, а не просто делаете ему выговор за то, что он поступил по-своему, а не по-вашему.

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

Разработчик пишет класс, который выглядит как что-то среднее между Factory и Container, затем называет его SomethingBuilder, где часть Something не связана с окончательно построенным объектом. Я, конечно, комментирую это, и скажем так, он принимает это на свой счет.

Многое зависит от того, как вы это прокомментируете. Сравнивать:

Этот класс выглядит как что-то среднее между Factory и Container, и он называется SomethingBuilder, который на самом деле не связан с окончательно построенным объектом.

к:

Интересно, может быть, стоит разделить это на Factory и Container следующим образом:

[.. небольшое количество псевдокода, чтобы продемонстрировать, что вы имеете в виду..]

Думаю, это даст нам [конкретное преимущество X].

Что вы думаете?

Кроме того, кажется, что Нечто на самом деле не раскрывает намерения, так как это скорее Другое. Может быть, переименовать его в Другое?

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

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

Он говорит, что у каждого свой стиль. Я не уверен, что это за стиль называть Factory a Builder, но неважно.

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

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

Не ждите, что он изменится, просто бросив ему словарные определения «Фабрика» или «Строитель»; объясните, как это улучшит ситуацию с объективной точки зрения. «Более элегантно» часто используется, но не очень хороший аргумент. Если вам трудно придумать лучшие аргументы, чем «более элегантный», тогда, может быть… это не так уж важно?

Честно говоря, я обычно делаю довольно тщательные обзоры кода.

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

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

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

Мне кажется здесь, что вы не тот высокий человек на тотемном столбе, так сказать. Таким образом, на самом деле есть только ограниченное количество вещей, которые вы можете сделать, и кажется, что вы их сделали:

1) Если команда не занимается PR/CR процессом, вы должны настаивать на его внедрении (вы это сделали).

2) Если люди штампуют PR, вы должны призвать их остановиться (вы сделали это).

3) Вы должны быть примером, и когда кто-то присылает вам плохой PR, вы должны прокомментировать его и объяснить, что не так (вы сделали).

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

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

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

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

Вот несколько практических идей для решения этой проблемы.

1. Попробуйте разделить комментарии обзора кода на объективные и субъективные моменты. Объективные точки включают наблюдения, такие как «здесь потенциальная ошибка» или «это не соответствует требованиям». Субъективные комментарии включают «есть лучший способ сделать это» или «это лучше назвать X». Часто бывает полезно прямо указать в комментариях разницу: «По моему мнению…» / «Подумайте о том, чтобы попробовать…». Я думаю, это помогает снизить обороноспособность.

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

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

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

5. Что касается вопросов более высокого уровня о том, как получить согласие PR, или о том, как разрешить конфликт из-за PR, это хорошие темы для обсуждения в ретроспективе (при условии, что ваша команда придерживается какого-то Agile-процесса).

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

Технический руководитель

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

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

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

коллега

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

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

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

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

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

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

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

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

Обсуждение именования

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

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

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

1. опишите вывод
2. описать бизнес-логику, содержащуюся в

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

saveDataInAthreadSafeAndDataConsistentManner(){...}

не полезно. Но

saveAddressDetailsAsTheresAConcurrentProblemWhenSavedWithEverythingElse()

ну, это полезное имя метода.

Но, в конце концов, рабочий код >> хорошо названный код, который не работает, и быстро созданный рабочий код >> медленно произвел хорошо названный рабочий код.

Другая мысль

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

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

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

Кроме того, ваша управленческая цепочка проигнорировала основы математики и психологии.

Что касается математики, если один модуль имеет 99%-й шанс отсутствия ошибок (я не использую достаточно модульных тестов), то вы можете рассчитать (1-p)^n, что 100 модулей вместе имеют шанс 37%-го поведения без ошибок. . Вероятность очень быстро идет против вас. Повышение «качества» до 99,5% улучшает систему чуть более чем на 60%.

Что касается психологии, кто хочет работать на или с людьми, которым все равно?