Вчера в нашем внутреннем рабочем пространстве Slack возник вопрос о том, как проводить хорошую проверку кода при запросах на вытягивание. Вопрос вызвал большой энтузиазм и несколько подробных ответов. Очевидно, что это важный вопрос, который волнует большинство из нас, и многие размышляли над ним много времени. Мы подумали, что это прекрасная возможность поделиться своим опытом и составить документ для использования в будущем, чтобы обеспечить хорошую культуру разработки в разных командах.
В этом посте будет рассказано, как стать лучшим «рецензентом кода». Мы не будем рассказывать, как лучше писать запросы на вытягивание - обратите внимание на эту публикацию в будущем. Этот пост совместно написали Сара Серусси, Кристиан Бревик, Андерс Ньёс Слинде, Магнус Даль и Микаэль Бревик.
Перейдем к советам!
1. Будьте вежливы
Важно сохранять дружелюбный тон и неформальный стиль письма. Типичный западно-норвежский терпкий сарказм не очень хорошо работает при проверке кода!
Отправитель мог потратить много времени и умственной энергии на написание кода. Это означает, что они могут чувствовать себя немного напряженными или неуверенными в содержании запроса на вытягивание. Их последующие «вложения» в работу будут высокими, и, глядя на финишную черту, они, вероятно, с нетерпением ждут, когда их код слит и попадет в руки пользователей как можно быстрее.
Вы, как рецензент, обладаете властью в этой ситуации, используйте ее осторожно и экономно. Будьте вежливы! Если есть затяжные обсуждения, которые ни к чему не приводят, переместите их на другие платформы, такие как Slack, с целью объединения запросов на вытягивание, а не затягивания процесса.
2. Найдите время, чтобы понять основную цель запроса на вытягивание.
Понимание намерения изменения важнее, чем его построчное чтение. Читая только код без какого-либо контекста, вы, вероятно, получите поверхностное представление о том, для чего предназначен код. Вы можете обнаружить «тривиальные» ошибки, такие как отсутствие точек с запятой, проблемы с именами, стилистические изменения и т. Д. Но гораздо сложнее выявить фундаментальные проблемы с самим решением и проблемами, которые оно пытается решить.
Для этого нам нужно понять назначение кода. Что это значит изменить или добавить? Что вызвало необходимость в запросе на вытягивание? Это означает, что рецензенту нужно делать больше, чем просто читать код.
Найдите время, чтобы полностью понять контекст и последствия патча. Внимательно прочтите описание запроса на вытягивание, попробуйте внести изменения самостоятельно (см. Совет № 5), задайте вопросы (см. Совет № 6) и соедините точки. Таким образом, мы можем общаться на более высоком уровне, чтобы предлагать альтернативные решения, находить логические ошибки или улавливать недопонимание. Это также поможет нам как команде быть более ориентированными на пользователя. Вместо того, чтобы просто комментировать код, мы можем комментировать решения и функциональность.
3. Предоставьте предложения по улучшению. Показывайте решения, а не проблемы.
Часто, когда вы работаете над исправлением ошибки или новой функцией, вы можете быть взволнованы и / или торопливы и почувствовать, что хотите отправить запрос на перенос как можно скорее. В других случаях вы могли бы работать над запросом на перенос в течение нескольких дней. В любом случае при работе с кодом очень легко упустить из виду опечатки, временные операторы журнала или неправильное именование переменных, поэтому процесс проверки так важен.
При обзоре кода может быть легко взглянуть на него критическим взглядом и почувствовать, что цель состоит в том, чтобы найти как можно больше ошибок. Хотя это нормально, важно сообщать об ошибках таким образом, чтобы это было значимо для отправителя запроса на вытягивание. Также легко перестать замечать собственный код, так что помните об этом, обнаруживая такие ошибки.
Многие платформы, такие как GitHub или Azure DevOps, имеют функцию, которая позволяет отправлять предложения в запросах на вытягивание. Это означает, что рецензент может давать предложения по изменению кода для незначительных ошибок или опечаток, вместо того, чтобы по умолчанию просто указывать на проблемы и оставлять их на усмотрение отправителя.
Например, называть вещи очень сложно, поэтому когда (вопрос времени) вы обнаружите, что наименования неудачный, отнеситесь к этому конструктивно и предложите альтернативу вместо критики, не помогая ситуации. В этих случаях функция предложения является золотой.
Если вы обнаружите более крупные ошибки или блоки кода, которые можно заменить чем-то совершенно другим, вы должны объяснить отправителю, о чем вы думаете, и предложить возможное решение. Это можно сделать лично, в Slack или в разделе комментариев запроса на перенос. Если возникает конфликт, вовлекайте других или подсчитывайте свои потери.
4. Избегайте стилистических, чрезмерно придирчивых комментариев.
У людей разные взгляды на то, как они хотели бы, чтобы их код выглядел. Однако обсуждение этого в разделе комментариев запроса на перенос может быть не подходящее время для этого. Такие вещи должны быть согласованы и автоматизированы задолго до того, как вы когда-нибудь дойдете до пулреквеста.
В начале проекта вы и ваши коллеги-разработчики должны установить правила того, какую автоматизацию вы хотите включить в свой проект, и всегда полезно иметь какой-нибудь инструмент форматирования. Это позволяет вам и людям, с которыми вы работаете, иметь общий набор правил, не задумываясь об этом позже. На самом деле не имеет значения, каковы правила, если вы все в них едины.
Следует иметь в виду, почему вы вообще смотрите на этот код? Это должно быть для того, чтобы привлечь больше внимания к задаче, но также и для обучения друг у друга. В этой дискуссии вопросы вкуса не так актуальны.
5. Не бойтесь проверять изменения на месте!
Самое замечательное в проверках кода с помощью запросов на вытягивание состоит в том, что они меняют контекст того, как вы смотрите на код. Это выводит вас из привычных настроек, которые предоставляет ваша IDE, и заставляет взглянуть на изменения в ином свете. Этот сдвиг может помочь вам заметить ошибки, которых в противном случае не было бы. Это верно как для человека, открывшего запрос на вытягивание, так и для рецензентов.
Но иногда возникают трудности с просмотром кода в веб-интерфейсе. Сам набор изменений может быть слишком большим. В этом случае вам может быть сложно ориентироваться в изменениях и понимать, какое влияние они оказывают. В других случаях логика может быть слишком сложной для простого чтения. Вы застреваете в попытках прочитать его построчно и «скомпилировать в своей голове», чтобы понять, что происходит.
В этих случаях лучший способ двигаться вперед - это обычно проверять изменения локально в вашей среде разработки. Это позволяет вам использовать предпочитаемый вами редактор или IDE для навигации по большим изменениям и запускать код, чтобы понять влияние или логическую сложность. Другими словами, компилируйте и запускайте код на своем компьютере, а не в голове.
Это может показаться препятствием для продуктивной работы разработчика или команды. Вы не хотите быть препятствием на пути к переменам. Но худший сценарий - обнаружение ошибки намного позже, которую можно было бы обнаружить при проверке кода. Чем больше времени уходит на поиск ошибки с момента написания кода, тем труднее вы можете попытаться исправить эту ошибку.
6. Задавайте (открытые) вопросы.
Можно посмотреть на пул-реквест и подумать о нем как о процессе проверки устойчивости кода. И в определенной степени - это так. Но это еще не все. Это также акт рассказывания историй и учета того, как ваш код оказался в том состоянии, в котором он находится.
Другими словами, код сам по себе может не рассказать всей истории, когда другие попытаются его понять. Одна его часть - это комментарии в коде, но они могут относиться только к пониманию незначительных деталей определенных файлов или определенных строк кода. Более широкую картину того, почему код, невозможно нарисовать, просто прочитав код, комментарии или историю репозитория.
Вытягивайте запросы и проблемы, а обсуждение, которое их окружает, заполняет важное пространство при попытке понять причину решений, принятых при разработке решений. Это означает, что очень важно вести реальный диалог по мере его развития.
Для вас как для обозревателя кода это означает, что если вы чего-то не понимаете, никогда не бойтесь задавать вопросы. Очень вероятно, что кто-то придет позже и задумается о том же, что и вы. Это означает, что вы поможете написать историю о том, почему что-то произошло, а не только о том, что произошло. И в то же время человек, открывший запрос на перенос, может подумать о том, как он решил конкретную проблему, и при необходимости сделать это иначе.
Важно отметить, что, хотя задавать критические вопросы - это нормально, постарайтесь сформулировать их так, чтобы обсуждение могло быть продуктивным. Открытые вопросы - отличный инструмент для использования здесь - не пытайтесь сдвинуть обсуждение в определенном направлении, которое принесет пользу только вам, или поставить людей на их место. Опять же, будь вежливым (см. Совет №1)! Открытые вопросы дают вам гораздо больше информации, чем просто подтверждение вашего собственного предвзятого мнения.
Вместо того, чтобы задавать закрытый вопрос вроде «Этот код беспорядочный. Действительно ли это хороший способ справиться с этим крайним случаем? », Дайте другому человеку возможность рассказать свою историю, спросив:« Я не совсем понимаю этот код. Вы можете объяснить, как это решает крайний случай? »
7. Оптимизируйте пропускную способность, а не привратник.
Всем будет лучше, если запрос на перенос будет объединен раньше, чем позже. Медленный процесс проверки кода, который превращается в устаревшие запросы на вытягивание, очень демотивирует как проверяющих, так и отправителей.
Если что-то выходит за рамки, разделите это на новый запрос на перенос или перенесите обсуждение в другое место, если его все еще можно объединить. Многие детали можно исправить позже, если код работает как единое целое и не нарушает общую архитектуру. Сосредоточьтесь на большом, а не на маленьком картофеле. Ищите очевидные ошибки и логические ошибки, более снисходительно относясь к незначительным проблемам.
Процесс проверки кода должен быть ценным уроком, а не использоваться в качестве средства контроля. Помните, что вы участвуете в этом вместе, как команда, и цель должна заключаться в том, чтобы держать мяч в покое. И ради поддержания мотивации команды, и для того, чтобы постоянно приносить пользу.
Узнайте, что работает для вас и вашей команды. Запишите свои рекомендации относительно того, как вы хотите проводить обзоры кода, и найдите общий консенсус. По крайней мере, поговорите о том, что, по вашему мнению, является хорошей проверкой кода.
Это наши 7 советов, как повысить эффективность проверки кода в вашей команде. Можете ли вы вспомнить что-нибудь еще, что сработало для вас, или что-то, что мы упустили?
Авторы Сара Серусси, Кристиан Бревик, Андерс Ньос Слинде, Магнус Даль и Микаэль Бревик .
