Практическое инженерное руководство

Не позволяйте рецензированию кода превращаться в праздники дедовщины

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

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

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

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

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

Как правильно относиться к пул-реквестам?

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

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

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

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

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

🔔 «Хотите больше подобных статей? Подпишите здесь."

Обратная связь - не ваш шанс проявить себя.

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

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

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

Иногда (то есть часто) мы выдаем собственное мнение за абсолютную истину. Думаю об этом. «Поля следует объявлять перед конструктором», «никогда не использовать одиночные элементы», «лучше всего…». Эти три утверждения суть мнения. Не правда.

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

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

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

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

Контрольный список для создания отличного запроса на вытягивание.

Убедись в том, что:

  • Ваш код строится (да…)
  • Модульные тесты обеспечивают достаточное покрытие кода
  • Изменения правильно реализуют / исправляют пользовательскую историю / ошибку
  • Код тестируется локально с данными, похожими на производственные.
  • Запрос на вытягивание правильно описывает сделанные изменения.
  • Предоставьте ссылку на историю пользователя / ошибку
  • Упоминание любых улучшений, не связанных с пользовательской историей / ошибкой.

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

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

и ваш контрольный список для проверки чужого кода…

Я рекомендую вам скопировать этот список в комментарий к запросу на перенос и отметить их один за другим.

  • Соответствует ли код методам решения?
  • Легко ли читать и поддерживать код?
  • Требуется ли для новых изменений обновленная документация?
  • Правильно ли обрабатываются крайние случаи и исключения?
  • Нет метода с оценкой цикломатической или когнитивной сложности выше 10.
  • Код проверяется локально
  • Код тестируется локально при запуске приложения.

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

Давайте оставаться на связи!

Получайте уведомления о похожих статьях, подписавшись на рассылку новостей здесь и посетите новый канал YouTube (@Nicklas Millard)

Подключиться к LinkedIn

Ресурсы для любопытных.