Часть I. Мифы

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

Миф: только старшие разработчики могут проводить проверку кода

Лучший способ стать лучшим разработчиком — это писать код. Однако второй лучший способ — это читать чужой код.

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

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

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

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

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

Так где ты ошибся....

1. Возможно, вы не написали соответствующие комментарии, объясняющие, что делает код.

Комментарии не должны объяснять очевидное. Комментарии должны относиться к отправленным и полученным сообщениям.

# BAD:
#
# This is a method that returns the sum of 2 or more numbers
def sum( *args )
 args
 .flatten
 .select { |a| a.is_a?( Numeric ) }
 .map( &:to_f )
 .reduce( :+ )
end
# GOOD:
#
# This is a method that returns the sum of its arguments
# as a Float. Arguments that are not Numerics or Arrays
# of Numerics will be ignored.
#
# e.g. you can sum a number and a range as follows:
#
# sum( 2, (1..5).to_a ) # returns 17.0
def sum( *args )
 args
 .flatten
 .select { |a| a.is_a?( Numeric ) }
 .map( &:to_f )
 .reduce( :+ )
end

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

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

2. Возможно, вам не удалось написать тесты.

Если написание хорошего комментария помогает вам понять код, который вы написали, то написание тестов заставит вас действительно его понять.

Если вы не можете написать модульные и/или интеграционные тесты, чтобы убедиться, что написанный вами код работает так, как вы предполагали, вам нужно исправить это (и быстро).

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

Предполагая, что вы используете что-то вроде потока Git или потока Github, вы будете использовать PR для объединения своей работы в основную ветку.

То, что вы пишете в PR-описании, так же важно, как и все остальное в PR. Ссылка на задачу в вашем инструменте управления проектами (вероятно, он уже имеет интеграцию с вашим инструментом репозитория кода). Выделите любые соответствующие комментарии, которые объясняют, почему вы приняли определенные решения. Добавьте больше деталей и ясности о том, почему X, Y, Z существуют или не существуют.

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

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

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

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

Однако последним возражением может быть то, что менее опытные разработчики или разработчики, не работающие над проектом, не поймут «домен».

Опять же, это почти наверняка неверно.

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

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

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

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

Если кто-то просматривает ваш запрос на вытягивание и не понимает предметную область, укажите ему следующее:

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

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

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

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

Так вот. Любой профессиональный разработчик может просмотреть ваш PR (при условии, что компания позволит им), если вы пишете хороший код и если какие-либо вопросы по домену (на которые нет ответов в коде, тестах или задачах и/или описаниях PR) решаются профессионалами которые действительно хорошо объясняют подобные вещи разработчикам.

Миф: я могу проверить свой собственный код, чтобы я мог сам закрывать запросы на слияние.

Это не только неверно, это опасно.

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

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

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

У вас максимум два глазных яблока. Так что добавьте их в свой код. Минимум четыре. Иногда больше.

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

Часть II. Как проводить проверку кода

Спринты, а не марафоны

Трудно просматривать код более 30 минут. Можно просмотреть 200–300 строк кода, но нельзя просмотреть более 500 строк кода.

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

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

Знайте цели...

  • Код без дефектов.
  • Код, обеспечивающий функциональность, для которой он был написан.
  • Код, соответствующий любым согласованным руководствам по стилю и/или стандартам, которые использует ваша команда.
  • Код, который не нужно рефакторить, чтобы использовать его в будущем (но будьте осторожны, чтобы не пытаться предсказать будущее, просто ищите осмысленные объекты с четко определенными интерфейсами).

Может быть полезно следовать контрольному списку, подобному этому (поскольку большая часть этого была отобрана из Интернета на протяжении многих лет, я попытаюсь вернуться и найти источники, чтобы предоставить соответствующие атрибуции):

Общие

  • Код работает? Выполняет ли он свою предназначенную функцию?
  • Код легко понять?
  • Соответствует ли он согласованным соглашениям о кодировании? (например, https://standardjs.com/rules.html и https://google.github.io/styleguide/htmlcssguide.html)
  • Это СУХОЕ?
  • Насколько это модульно?
  • Можно ли удалить какие-либо глобальные переменные и заменить их чем-то лучшим (почти всегда есть что-то лучшее)?
  • Есть ли закомментированный код? (не должно быть ни одного)
  • Существуют ли условные выражения там, где их можно было бы избежать (особенно внутри других условных выражений)?
  • Есть ли скрытые объекты, которые могут быть обнаружены (есть ли целые модули, которые могут быть обнаружены, вы будете удивлены, как боятся некоторые разработчики, когда дело доходит до создания новых текстовых файлов или папок)?
  • Можно ли заменить какой-либо код (хорошо протестированными) библиотечными функциями?
  • Удалили ли вы какой-либо код регистрации или отладки, который вы написали при разработке функции?
  • Есть ли в PR какие-либо ненужные коммиты?
  • Перебазирована ли ветка PR относительно основной ветки (например, она содержит только коммиты «впереди», а не «после»)?

Безопасность

  • Подходит ли код для защиты?
  • Проверяются ли все вводимые данные (на предмет правильного типа, длины, формата и диапазона) и при необходимости очищаются?
  • Если используются сторонние утилиты, выявляются ли возникающие ими ошибки, а затем корректно обрабатываются (не перехватываются, а затем отбрасываются)?
  • Проверяются ли выходные значения и кодируются ли они?
  • Обрабатываются ли недопустимые значения параметров?

Документация

  • Существуют ли комментарии, описывающие назначение кода?
  • Все ли публичные/интерфейсные функции закомментированы?
  • Описано ли какое-либо необычное поведение или обработка крайних случаев?
  • Документировано ли использование и функции сторонних библиотек?
  • Объясняются ли структуры данных и единицы измерения?
  • Есть ли неполный код? Если да, следует ли его удалить или пометить подходящим маркером, например TODO:? (Обратите внимание, что я предпочитаю удалять неполный код, а не использовать TODO:, но если вы его используете, убедитесь, что вы следуете этому стилю: «https://google.github.io/styleguide/htmlcssguide.html# Пункты действий")
  • Есть ли в запросе на вытягивание хорошо написанное описание, объясняющее внесенные изменения?
  • Есть ли в репозитории файл README, объясняющий назначение базы кода и способы ее развертывания в вашей среде разработки, запуска тестов и внесения изменений?

Тестирование

  • Можно ли тестировать код? то есть он не прячет зависимости или не может предоставить достаточно инструкций или, если это уместно, фактических тестовых данных, чтобы обеспечить репликацию состояния?
  • Существуют ли тесты и являются ли они исчерпывающими? т.е. 100% покрытие кода (как эмпирическое, но не фактическое правило) для всех моделей, сервисов, компонуемых функций (например, директив, компонентов и т. д.) и, при необходимости, написан набор функциональных тестов (кликабельно/ функциональность, доступная для прослушивания, обычно является хорошим кандидатом для функционального тестирования)?
  • Проводили ли вы кроссбраузерное/девайсное тестирование?
  • Действительно ли модульные тесты проверяют, выполняет ли код предполагаемую функциональность?
  • Превышают ли юнит-тесты, которые сосредоточены на неудачах тестирования, количество успешных тестов как минимум в 4:1?
  • Можно ли отрефакторить тестовый код? Он слишком хрупок или требует ручной настройки для проверки? Есть ли какой-нибудь тестовый код, который можно было бы заменить использованием существующего API или записью из работающего API?

Инструменты для проверки кода

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

Единственные другие инструменты, которые могут вам понадобиться:

  • твой любимый редактор
  • правильная среда разработки.

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

  • перетащите код на свой ноутбук
  • установите deps, используя информацию, указанную в README
  • запустить тесты
  • запустить систему локально.

Если кто-то скажет вам, что вы не можете провести его PR по «какой-то причине», считайте это серьезным сигналом опасности. Обычно это плохой знак.

Часть 3. Заключение

Итак, у вас есть это. Код-ревью доступен каждому.

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

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