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

Перед обзором

  • Проверьте, есть ли в запросе на вытягивание хорошее описание изменений. Это должно по крайней мере объяснить проблему и решение. Чем дольше, тем лучше. Лучше всего, если изменения содержат ADR или ссылаются на проектную документацию.
  • Проверьте количество добавленных и удаленных строк. Если запрос на вытягивание слишком велик, подумайте о том, чтобы предложить автору разделить и отправить отдельные PR. В некоторых случаях может быть много измененных строк из-за новых правил форматирования или переноса кода. Если такие изменения не вносятся в отдельные коммиты, обнаружить важные изменения может быть сложнее. Предложите автору зафиксировать изменения форматирования отдельным коммитом.
  • Цените, если PR может быть рассмотрен от фиксации к фиксации.
  • Удалите изменения от автора, чтобы исключить предвзятость, потому что автор может обрабатывать код. Например; Если автор является старшим инженером, можно предвзято относиться к тому, чтобы не искать очевидные ошибки или колебаться с предложением тривиальных исправлений.
  • Выделите больший временной интервал для обзоров, содержащих изменения в незнакомых проектах/модулях. Используйте дополнительное время, чтобы зафиксировать контекст — текущее состояние кода и проблему, которую призван решить PR.

Во время обзора

  • Начните с обзора на высоком уровне, прежде чем вдаваться в мелкие детали. Имеет ли смысл предлагаемое решение? Есть ли принципиальные недостатки в подходе? Если это так, сначала оставьте комментарий об этом и обсудите это с автором, прежде чем углубляться в детали.
  • После согласования высокоуровневого подхода — проверяйте детали, начиная с изменений реализации, а затем юнит/интеграционные тесты.
  • Ищите способы предлагать комментарии, которые упрощают решение, если код не легко читать/понимать. Код, который нельзя понять сейчас, будет труднее понять через год.
  • Ищите потенциальные пограничные случаи, которые изменения могут не обработать.
  • Если изменения происходят в модулях/классах, у которых есть известная техническая задолженность, предложите улучшить код и оплатить задолженность (частично) всякий раз, когда это имеет смысл. Таким образом, технический долг можно будет выплачивать (или держать под контролем) постепенно.
  • Если изменения вводят новую стороннюю зависимость, проверьте ее лицензию. Также проверьте, есть ли у зависимости известные уязвимости безопасности или проблемы с производительностью.
  • Проверьте, влияют ли какие-либо новые добавленные зависимости на размер вывода (пакета).
  • Проверьте, не нарушают ли изменения обратную совместимость. Например, изменения в общедоступных API должны либо иметь версию, либо иметь только добавочные (необязательные) изменения.
  • Проверьте, не могут ли изменения привести к уязвимостям безопасности, таким как несанитизированный пользовательский ввод или необработанные ошибки времени выполнения, которые могут привести к утечке внутренних данных клиенту.
  • Проверьте изменения для любых операций с интенсивным использованием ЦП или ввода-вывода, которые могут снизить производительность, когда система находится под нагрузкой.
  • Проверьте, правильно ли изменения обрабатывают потенциальные исключения.
  • Наконец, просмотрите код, который не изменился. Например, изменения могли ввести новую вспомогательную функцию, и в коде могут быть другие места, где ее можно использовать повторно.
  • Не проверяйте стиль/форматирование кода. Это работа линтера, а не разработчика.

После обзора

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

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