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