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

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

Часть этой работы заключалась в просмотре запросов на слияние (PR) вместо их написания. Вот 15 вещей, которые я узнал о PR-обзорах.

1. Сосредоточьтесь на больших вещах и не парьтесь по мелочам

Ищите фундаментальные недостатки, а не мелочи. Вы понимаете, чего пытается добиться пиарщик? Вы понимаете, почему оно это делает? Функция работает корректно? Хорошо ли он сочетается с остальной частью системы? Пытается ли он достичь поставленной цели? Не вызовет ли это какой-нибудь непредвиденный баг?

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

2. Будьте последовательны

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

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

3. Оставьте подробный отзыв

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

  • В чем проблема и почему проблема проблемой
  • Предлагаемое решение
  • Ссылки на внешние ссылки

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

4. Дайте им знать, если вы просто думаете вслух

Иногда вы хотите сделать комментарии к PR, которые являются просто открытыми мыслями.

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

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

5. Включите ободряющие комментарии

Если какой-то фрагмент кода был сделан хорошо, добавьте это в качестве PR-комментария, например: «Это было хорошо написано!» или «Отличное использование техники ____!»

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

6. Установите ожидаемое время отклика

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

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

7. Не рецензируйте PR, если вы не имеете на это права

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

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

8. Избегайте длительной переписки

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

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

9. Код был объединен — все равно проверьте

Иногда код сомнительного качества необходимо объединить из-за необходимости выпуска функции. Может быть, у вас не хватило времени, чтобы прочитать/просмотреть его, может быть, кто-то еще одобрил PR, но также пропустил много проблем.

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

10. Ответственность за изменение должна нести кодировщик.

Давайте проясним: в конечном счете ответственность за свои изменения должен нести кодер, а не рецензент. Рецензент также должен читать код, конечно, но он не должен нести полную ответственность за код.

Таким образом, это будет стимулировать кодера:

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

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

11. Личные разговоры всегда лучше, чем злые сообщения

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

12. Дайте обзоры, адаптированные к набору навыков разработчика

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

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

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

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

13. Я знаю, что что-то не так… но я не могу этого объяснить…

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

14. Будьте тверды в своих убеждениях

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

15. Используйте смайлики

На них приятно смотреть. Конечно, ваши PR-обзоры станут веселее.

Вывод

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

PR-обзоры — это возможность учить, учиться и обсуждать код. Это не должно быть мучительным или бессмысленным занятием. Постарайтесь, чтобы это было продуктивно, конструктивно и (надеюсь) весело!

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

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