Шаблон запроса на вытягивание для победы!
Первые шаги
Более года назад наша команда Android в RSQ Technologies представила шаблон описания запроса на вытягивание в процесс проверки кода. Это продвинуло нас еще на один шаг к более прозрачной и плавной разработке приложений. На тот момент нашему приложению было уже два года, и в нем были новые функции в разработке и постоянное сопровождение. В ходе этого процесса разработки мы столкнулись с множеством проблем, связанных с отсутствием знаний об определенных частях кодовой базы. Код становился все труднее и труднее понимать с каждым новым представлением, функцией или модулем.
Мы решили использовать шаблон описания запроса на вытягивание, чтобы помочь нам понять изменения, внесенные в приложение, найти ошибки и указать на запахи кода.
В этой статье я хочу показать вам наш шаблон, который мы использовали в течение года. Далее я расскажу о том, что сработало, о том, что не сработало, и об изменениях, которые мы внесли. Если вас просто интересует окончательная форма нашего шаблона - просто перейдите в раздел Текущий шаблон проверки кода .
С чего мы начали
Наш шаблон запроса на вытягивание органично вырос. Некоторые из пунктов были основаны на статьях, которые мы читали в то время, некоторые из них были добавлены позже на нашей еженедельной ретроспективной встрече команды.
# Description, Motivation, and Context # # What is the destination branch of this PR? # # How Has This Been Tested? # # Screenshots (if appropriate): # # Types of changes # [ ] Bug fix (non-breaking change which fixes an issue) [ ] New feature (non-breaking change which adds functionality) [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) # Checklist: # [ ] I merged current develop to this branch. [ ] My change requires a change to the documentation (confluence test paths, code style etc.) [ ] I have updated the documentation accordingly. [ ] My change requires a change to the UI tests. [ ] I have updated UI test accordingly. [ ] I unit tested new methods. [ ] I checked linter on local. [ ] I checked unit tests on local. # Is additional manual testing needed? # # Files with major changes / what is worth paying more attention to? # # Which variances have been affected # [ ] variant 1 [ ] variant 2 [ ] variant 3 [ ] variant 3
Что сработало, а что нет
- Описание, мотивация и контекст
Работал? Определенно да.
Один из самых важных пунктов в списке. Когда рецензенты начинают читать пул реквест, они получают самую важную информацию об изменениях.
- Какие изменения?
- Почему вы выбрали это решение?
- Какова была цель этой задачи?
- В чем заключалась основная идея кода?
Вся информация, указанная в первом пункте, дает рецензенту четкое представление об истории этого изменения. Намного легче указать на болевые точки в коде, если вы поймете, что послужило причиной этого изменения.
2. Какова конечная ветвь этого PR?
Работал? да.
На первый взгляд это может показаться глупым. Вы можете сказать, что место назначения запроса на перенос, вероятно, отображается где-то в верхней части страницы (это место зависит от того, что вы используете Github, BitBucket и т. Д.). Этот вопрос был добавлен в шаблон, чтобы создатель PR обратил внимание и перепроверил пункт назначения.
У нас было несколько проблем после слияния ветки. Затем понял, что он был объединен с разработкой по умолчанию, а не с ветвью выпуска или веткой функций. Добавление этого вопроса в шаблон решило проблему.
3. Как это было проверено?
Работал? да.
В нашей команде Android на этом этапе мы в основном сосредотачиваемся на эмуляторах и физических устройствах, используемых для тестирования кода. Иногда было важно убедиться, что функция протестирована как на телефоне, так и на планшете или просто на телефоне с небольшой плотностью пикселей.
Когда запрос на вытягивание вносит более крупные изменения в приложение, эта часть также состоит из проверенных путей тестирования, введенных случайных данных и т. Д.
4. Скриншоты (при необходимости)
Работал? Определенно да.
Эта идея пришла к нам с Совершенствованием процесса представления PR от Атаула Мунима. Эта статья была представлена на одной из наших еженедельных встреч по обмену знаниями, и сразу после ее окончания мы сразу же решили, что должны использовать ее в процессе проверки кода!
В среде Android не так быстро оформить заказ и отобразить чье-то конкретное представление, в котором было внесено изменение. Когда проект достаточно большой, проверка новой ветки, компиляция кода и щелчок по приложению прямо в том месте, где были внесены изменения, могут занять десятки минут. Также сложно визуализировать изменения, просто читая файлы XML на Android или HTML / CSS в Интернете.
Когда запрос на вытягивание состоит из снимков экрана, вы можете сразу определить, правильно ли выглядит визуальная часть или нет. что-то не так.
5. Типы изменений
Работал? Не совсем.
[ ] Bug fix (non-breaking change which fixes an issue) [ ] New feature (non-breaking change which adds functionality) [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
Первая чушь в списке. Три флажка, которые должны дать рецензенту больше информации о задаче, но в конечном итоге они были неинформативными. В большинстве случаев это было дублирование информации из первого пункта Описание, мотивация и контекст.
6. Контрольный список
Работал? В большинстве случаев - да.
[ ] I merged current develop to this branch. [ ] My change requires a change to the documentation (confluence test paths, code style etc.) [ ] I have updated the documentation accordingly. [ ] My change requires a change to the UI tests. [ ] I have updated UI test accordingly. [ ] I unit tested new methods. [ ] I checked linter on local. [ ] I checked unit tests on local.
Все эти пункты были добавлены только для того, чтобы напомнить разработчику, подготовил ли он / она код до того, как он был готов к слиянию.
Контрольные списки рекомендуются в большинстве шаблонов запросов на вытягивание.
7. Требуется ли дополнительное ручное тестирование?
Работал? да.
Разработчик может попросить других в своей команде проверить функцию и протестировать ее самостоятельно перед объединением. Он в основном используется в ветвях функций или ошибок для крайних случаев.
8. Файлы с существенными изменениями / на что стоит обратить внимание?
Работал? Хм… Иногда?
Этот момент был добавлен где-то в середине процесса, когда новые запросы на вытягивание приходили с огромными изменениями, но в основном с миграцией и переименованием файлов. В таких случаях мы хотели указать рецензенту, чтобы он сосредоточился на наиболее важных файлах, в которых выполнялась большая часть логики.
9. Какие варианты были затронуты
Работал? да.
Этот момент может быть весьма специфичным для разработки под Android. Вариант - это тип сборки - он определяет, какое приложение собирается из кодовой базы. В нашем проекте мы создаем несколько приложений (более четырех) из одной кодовой базы. Очень важно знать, какие приложения будут затронуты этой модификацией.
Это может сработать и для вас, если ваш проект похож.
Следующие шаги
После года работы с этим шаблоном мы решили внести некоторые изменения. Мы начали чат в нашем рабочем приложении для обмена сообщениями, чтобы обсудить все, что нам нравится и не нравится в шаблоне - что мы обычно пропускаем, что является наиболее важным и какие вопросы не были достаточно точными. Пришло время освежить идею и подробнее поговорить о новых контрольных точках, вопросах. Также убрал несколько бессмысленных точек.
Одной из статей, которая также подтолкнула нас к действию и внесению некоторых изменений, была Контрольный список проверки кода, чтобы сосредоточить внимание на важных частях от Микаэлы Грейлер. Все новые пункты, добавленные к обзору кода, основаны на предложениях, предложенных в ее статье. Спасибо!
1. Ссылка на ресурсы (макеты, описание изменения)
Когда за изменением стоит больше мыслей, полезно иметь ссылки на ресурсы, которые дадут вам более широкую перспективу по проблеме.
2. Требуются ли для этого изменения обновления вне кода, например обновление документации, конфигурации? Если да, было ли это сделано?
Ранее этот вопрос был скрыт в контрольном списке. Мы превратили его в вопрос, чтобы создатель PR мог задуматься над ним глубже.
3. Можно ли проверить код? Если нет - почему?
Мы наткнулись на некоторые проблемы с тестируемостью в нашей кодовой базе. Поэтому мы хотели, чтобы каждый программист переосмыслил свой код в случае тестируемости и начал небольшое обсуждение, есть ли возможность дать тестам еще немного времени.
4. Существуют ли аналогичные функции в базе кода? Если да, то почему эта функция не используется повторно?
Подобно предыдущему пункту - нам нужно было место, чтобы обсудить, была ли такая функция уже написана где-то в базе кода и, возможно, она может можно использовать повторно.
Текущий шаблон обзора кода
# Description, Motivation and Context # Link to the resources (mockups, change description) # What is the destination branch of this PR? # How Has This Been Tested? # Screenshots: # Does this change require updates outside of the code, like updating the documentation, configuration? If so, was this done? # Do reviewers should manually test your changes? # Is the code testable? If not - why? # Does similar functionality already exist in the codebase? If so, why isn’t this functionality reused? # Checklist: [ ] I unit tested new methods. [ ] I checked linter on local. [ ] I checked unit tests on local. # Which variants have been affected [ ] variant 1 [ ] variant 2 [ ] variant 3 [ ] variant 4 # Files with major changes / what is worth paying more attention to? (Delete this point if it is not needed)
Стоит ли использовать шаблон запроса на вытягивание?
Да! Если есть какой-то совет, которым я хотел бы поделиться с вами в этой статье - уделите больше внимания обзорам кода. Когда рецензенты знают контекст, понимают поток данных и виден конечный результат, легче обнаружить проблемы в коде.
Сделайте свой собственный шаблон описания запроса на вытягивание, добавьте Проверки PR Bitbucket, автоматизируйте процессы с помощью непрерывной интеграции, попробуйте Опасность. Сделайте свой путь к лучшей проверке кода.
Хорошо подготовленный обзор кода создает хорошо проверенный код. Что, в конце концов, делает приложения лучше.
