Как правильно просматривать запросы на слияние
6 июня 2022 г.Звучит легко, правда? Проверка запросов на вытягивание может и должна быть легкой… но проверяются ли они должным образом?
Снова и снова инженеры поднимают пулл-реквесты, за которыми следует «одобрение» коллег. В некоторых случаях парное программирование не выполнялось, запросы на вытягивание относительно велики, а вспомогательные модульные тесты отсутствуют.
Вам это знакомо? Если да, читайте дальше и делитесь своими мыслями в комментариях!
Почему это происходит?
Чтобы понять, как правильно проверять запросы на вытягивание, нам нужно знать, почему запросы на вытягивание не проверяются должным образом. Есть много причин; однако наиболее распространенные из них связаны с нехваткой времени и/или давлением со стороны руководства.
Поэтому давайте напомним себе, что нас нанимают как профессионалов для написания кода хорошего качества. Возьмем аналогию с врачами из «Чистого кода дяди Боба»:
<цитата>Чтобы пояснить этот момент, что если бы вы были врачом и у вас был пациент, который потребовал бы, чтобы вы прекратили все это глупое мытье рук перед операцией, потому что это занимает слишком много времени? Ясно, что пациент — босс; и все же врач должен категорически отказаться подчиниться. Почему? Потому что врач знает больше, чем пациент, о рисках заболеваний и инфекций. Со стороны врача было бы непрофессионально (не говоря уже о преступном) подчиняться пациенту.
Со стороны программистов также непрофессионально подчиняться воле менеджеров, которые не осознают опасности создания беспорядка.
Короче говоря, у нас есть обязанность обеспечивать надлежащую проверку запросов на вытягивание.
Почему мы отправляем запросы на вытягивание?
Почему мы отправляем запросы на вытягивание? Думаем ли мы когда-нибудь о том, почему?
Есть много веских причин, но, проще говоря, мы отправляем запросы на включение кода, чтобы проверить его правильность и обеспечить качество. Запросы на включение также имеют дополнительное преимущество, помогая новым и существующим разработчикам изучать постоянно растущую кодовую базу, а также приобретать навыки работы с новыми инструментами и технологиями.
Отлично! Если мы должным образом рассмотрим запросы на вытягивание, мы получим доступ ко всему этому полезному!
Итак, как правильно проверять запросы на вытягивание?
Держите запросы небольшими
Нет ничего хуже, чем просматривать большой запрос на вытягивание.
Небольшие запросы на вытягивание фокусируют наш обзор и помогают нам легко выявлять любые проблемы/улучшения. Кроме того, разработчики могут испугаться, столкнувшись со «стеной кода». Если наши запросы будут небольшими, разработчики с большей вероятностью захотят проверить наш код.
Две пары глаз лучше, чем одна!
Наличие как минимум двух рецензентов очень важно для обмена знаниями. При проверке кода рецензенты могут видеть комментарии друг друга, что позволяет им улавливать знания друг друга.
Наличие более одного рецензента также помогает снизить риск пропуска проблем.
Соответствует ли код стандартам кодирования?
Стандарты кодирования помогают обеспечить согласованность кода. Наличие единого источника достоверной информации помогает при написании и проверке кода.
При написании кода разработчики, как правило, более эффективны, поскольку они могут сосредоточиться на логике кода, а не беспокоиться о его стиле.
При просмотре не возникает споров, если кто-то не придерживался стандартов кодирования. Например, если для имени переменной используется змея_case, когда стандарты кодирования предписывают использовать нижний регистр CamelCase, достаточно комментария со ссылкой на стандарты.
Хорошо ли читается код?
Если нет, как его можно улучшить?
Ясность кода — один из самых важных и часто упускаемых моментов, когда дело доходит до рецензирования кода. Именование всего в проекте, включая переменные, классы, функции, папки и т. д., должно объяснять, что делает код, не требуя комментариев. В идеале он должен читаться как книга.
Например, рассмотрим следующие имена переменных:
Плохо: time = 4 # прошедшее время в днях
Хорошо: elapsed_time_in_days = 4
Второй пример является семантическим, и его легко понять, где бы он ни упоминался.
В качестве другого примера рассмотрим следующие имена функций:
Плохо: function processUser(user) { ... }
Хорошо: function addUserToMailingList(user) { ... }
Мы понятия не имеем, что делает первая функция; он может обрабатывать пользователя несколькими способами. Вторая функция намного лучше, так как она сообщает нам, что делает код внутри функции, без необходимости копать глубже — мы знаем, что она добавит пропущенного пользователя в список рассылки.
Не повторяйтесь (держите СУХОЙ)
Убедитесь, что код не повторяется. Повторяющийся код — идеальная среда для появления ошибок. Если код повторяется, извлеките код в функцию.
Есть ли ошибки в логике?
Убедитесь, что логика разумна и имеет смысл. Например:
- Все ли пути учтены?
- Что произойдет, если ожидается, что переменная не будет нулевой, но имеет значение?
- Делает ли код то, что он должен делать?
Есть ли адекватные тесты для поддержки нового кода?
Убедитесь, что модульные тесты написаны для поддержки нового кода. Кроме того, проверьте, что охватывают модульные тесты. Охватывают ли они приемлемое количество сценариев?
Стоит отметить, что написание тестов перед написанием кода TDD (разработка через тестирование) является хорошей практикой.
Наконец-то будь милым!
Важно быть приятным и полезным при рассмотрении запроса на вытягивание. Постарайтесь, чтобы ваши комментарии касались кода, а не разработчика.
Будьте полезными, предложив альтернативные решения и объяснив, почему эти решения могут быть лучше. Например:
Плохо. Почему вы вернули объект целиком? Нам не нужно возвращать все эти данные из API.
Хорошо. Клиенту нужны только свойства id и name. Поэтому мы можем обновить этот код, чтобы он возвращал только то, что необходимо. Это также дает дополнительное преимущество, заключающееся в уменьшении размера полезной нагрузки и снижении риска утечки конфиденциальных данных.
Вывод
Когда вы просматриваете свой следующий запрос на вытягивание, руководствуйтесь здравым смыслом, проверяйте код должным образом и помните, что нужно быть вежливым!
Первоначально опубликовано здесь
Оригинал