Давать и получать отзывы сложно, но у всех нас одна цель: стать лучше

Проверка кода - дело сложное. Их бывает трудно отдать, и они могут потребовать много энергии.

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

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

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

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

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

Общий обзор

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

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

Запустите код

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

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

Ознакомьтесь с масштабами изменений

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

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

Оставьте комментарии по архитектуре

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

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

Оставьте комментарии об окружающем коде

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

Я также видел код, который отлично смотрелся в diff. Представьте себе общий результат, который был total + tax, и какой-то защитный код, чтобы налог был 0, когда он не был определен. Это кажется разумным, но это было в итоговом компоненте. Уже был метод промежуточных итогов и выходные данные, поэтому я посмотрел на метод getTotal и обнаружил, что налог уже добавляется, хотя ничего этого не было в коде, который изменился. Я обнаружил это, исследуя почему необходимо внести именно это изменение. Оказалось, что на это повлияла уже существующая ошибка. Решение было переработано, чтобы исправить эту ошибку и удалить код, который заставил меня поискать.

Я мог бы упомянуть много таких случаев. Если вы заметили изменение, в котором не уверены, приступайте к расследованию. Если вам интересно, было ли необходимо изменить XYZ на ABC в тестовом примере, верните его и проверьте. Посмотрите на код. Снуп. Не просто смотрите на вещи в зелени.

Относительно этих последних двух шагов

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

Многие компании и команды борются с обменом знаниями. Я думаю, что хорошая проверка кода помогает. Он не только делится знаниями, фактически видя и понимая код постепенно в изменениях, но и дает вам конкретный Rolodex о том, кому и какие вопросы задавать. Также помогает то, что вы видели что-то, что вы можете вспомнить в другой работе. Это поможет с дублированием задач. Это может помешать вам выполнять дополнительную работу или просто дать вам кого-то проконсультироваться перед выполнением задачи.

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

Просмотр кода

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

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

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

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

Тесты - это настоящий код

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

О чем нужно помнить

Объем

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

Правило бойскаутов

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

Конкретно укажите, что необязательно

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

Не говорите расплывчато

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

Не налетай

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

Всегда приводите примеры

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

Попробуйте предлагаемые изменения для простых вещей

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

Не откладывайте обзор без причины

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

Если есть отказ, автор побеждает

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

Не обходите вниманием обзоры

Если вы выполнили пару раундов проверки кода и он становится лучше, оставьте все как есть. Не оставляйте новые комментарии к новому, что вы видели, и не пытайтесь сделать код на 100% идеальным.

Будьте вежливы

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

Корректура

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

Дополнительная информация

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

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

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

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

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

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