как делать код ревью
Как правильно делать код-ревью?
В жизни разработчика наступает момент, когда он начинает проводить код-ревью. Как правило, это один из признаков роста программиста: к его мнению начинают по-настоящему прислушиваться и доверять более серьезные задачи. Конкретный момент, когда разработчику предлагают заняться ревью зависит от структуры компании: есть команды, где это могут делать более-менее все по истечению некоторого времени работы, а есть такие, где процесс ревью целиком лежит на самых старших и опытных коллегах.
Вместе с тем, редко кто рассказывает о том, каких принципов необходимо придерживаться для проведения качественного ревью: каковы главные цели этой процедуры, за чем смотреть в первую очередь, насколько быстро его нужно проводить.
Всвязи с этим, «Руководство компании Google по проведению ревью» выглядит очень ценным документом, перевод первой части которого и представлен далее. Переводы остальных частей выйдут позже отдельными постами. Стоит отметить, что это адаптированный перевод, не все переведено слово-в-слово, во имя более русских формулировок и предложений.
CL: «changelist» — список изменений кода, отправленный в систему контроля версий на ревью. Аналог Pull Request в GitHub или Merge Request в GitLab.
1. Стандарты код-ревью
Главная цель проведения ревью — улучшение состояния кодовой базы компании Google. Все инструменты и средства, используемые для проведения ревью, направлены именно на достижение этой цели.
На пути к достижению озвученной цели приходится идти на ряд компромиссов.
Во-первых, разработчики должны иметь возможность делать свои задачи. Если вы не принимаете никаких изменений в кодовую базу, то она никогда и не улучшится. В случае, когда ревьюер, делает любые изменения слишком сложными, разработчики просто не заходят ничего делать в будущем.
С другой стороны, именно ревьюер несет ответственность за качество изменений в CL и следит за тем, чтобы состояние кодовой базы со временем не деградировало. Это непростая задача, поскольку часто код проекта ухудшается посредством мелких изменений на протяжении некоторого периода времени. Это ощущается особенно остро, когда на команду давят сроки и качество в мелочах приносится в жертву.
Ревьюер несет ответственность за тот код, который он ревьюит. Он должен быть уверен, что кодовая база остается консистентной, поддерживаемой, и отвечает все другим принципам из «За чем необходимо следить в ревью».
Таким образом, мы приходим к следующему правилу, задающему стандарт проведения ревью:
Ревьюер должен принять CL тогда, когда он улучшает общую кодовую базу проекта, даже если CL не идеален.
Это самый главный принцип проведения ревью.
Конечно, есть некоторые оговорки: если CL добавляет в код фичи, которые ревьюер не хочет видеть в проекте и не считает необходимыми, то он может не принимать изменения, даже если они улучшают кодовую базу.
Ключевым аспектом здесь является осознание того, что идеального кода не бывает: бывает код, который может быть лучше. Ревьюер не должен требовать совершенства в каждом отдельном небольшом кусочке CL перед тем, как принять изменения, он должен уметь находить баланс между улучшениями в CL как в коде, и важностью тех изменений, которые этот код несет. Вместо того, чтобы требовать совершенства, ревьюер должен стремиться к последовательному улучшению. CL, который улучшает поддерживаемость, читаемость и понятность кода не должен задерживаться в ревью на несколько дней или недель, по той причине, что он не идеален.
При этом ревьюер может свободно оставлять комментарии, поясняющие те моменты, которые могут быть сделаны лучше, однако, не имеют решающего значения. Помечайте такие комментарии определенным префиксом, например, «Nit: «, чтобы автор знал что эта деталь не обязательна к выполнению и ее реализация остается на его усмотрение.
Замечание: данный документ не оправдывает изменений в CL, которые ухудшают состояние кодовой базы проекта. Единственным исключением являются экстренные случаи.
1.1. Менторство
Код-ревью может также нести образовательный смысл, обучая разработчиков новым знаниям о языке, фреймворке или общих принципах написания кода. Всегда хорошо, если вы оставляете комментарий, который учит разработчиков чему-то новому. Кроме того, общие знания позволяют улучшать кодовую базу. Помните о том, что если ваш комментарий несет только образовательный смысл и не содержит критичных требований (по описанным в данном документе стандартам), помечайте его «Nit: » или прямо пишите в комментарии что он не обязателен к выполнению.
1.2. Принципы
1.3. Разрешение конфиликтов
В случае возникновения конфликтных ситуаций, прежде всего, стоит постараться прийти к консенсусу, основываясь на правилах, описанных здесь: Руководство для авторов CL и здесь Руководство для ревьюеров.
Если прийти к согласию не удается, то ревьюеру и автору стоит назначить очную встречу или видеозвонок: это будет эффективнее войны в комментариях. В таком случае, постарайтесь зафиксировать результаты обсуждения в комментариях к CL, для будущих читателей.
Если ситуация все равно не разрешилась, то нужно попробовать эскалировать конфликт — вы можете обсудить вопрос командой, узнать мнение первоначального автора кода, в который вносятся изменения, или спросить совета у более опытных коллег. Не позволяйте CL «зависнуть» просто потому, что ревьюер и автор не могут прийти к согласию.
2. На что обращать внимание в ревью
Замечание: Всегда учитывайте Стандарты код-ревью при рассмотрении каждого из принципов.
2.1. Дизайн (структура)
Самое важное, на что стоит обратить внимание в ревью — это дизайн CL в целом. Как взаимодействует код в рамках CL? Где находятся изменения: в общей кодовой базе или в вынесены в отдельную библиотеку? Насколько хорошо они интегрируются с остальными компонентами системы? Подходящее ли сейчас время для данных изменений?
2.2. Функциональность
Делает ли CL то, для чего он задуман? Насколько хорошо продуманы изменения для пользователей? При этом под «пользователем» понимается как конечный пользователь (если его затрагивают изменения), так и разработчики, которые будут использовать код в дальнейшем.
Предполагается, что разработчики достаточно хорошо тестируют свой код и на момент передачи в ревью он работает корректно. Вместе с тем, с точки зрения ревьюера, вы должны думать о граничных случаях, проблемах конкурентности, представлять себя на месте конечного пользователя, а также не забывать о багах, которые можно обнаружить просто читая код.
Существуют ситуации, когда может потребоваться дополнительная проверка CL: например, когда изменения касаются пользовательского интерфейса и через чтение кода сложно представить как это затронет конечного пользователя. В подобных ситуациях вы можете попросить разработчика сделать небольшое демо с новым функционалом, если запустить этот код самому представляется затруднительным.
Еще один случай, когда стоит отнестись к ревью более пристально — это наличие в коде CL параллельности в том или ином виде. Главные проблемы, которые может привнести параллельность — это дедлоки и гонки. Эти проблемы бывают очень трудноуловимыми, поэтому, необходимо чтобы и ревьюер и разработчик отнеслись к данному коду внимательнее. (Сложность ревью также является веской причиной избегать моделей конкурентности с возможными гонками и дедлоками).
2.3. Сложность
Проверяйте сложность кода на всех уровнях CL — в отдельных строках, функциях, классах. Слишком высокая сложность означает, что во-первых, нельзя быстро понять как работает код, во-вторых, при внесении изменений наверняка появятся баги.
Обычный случай, в результате чего код получается слишком сложным, это когда разработчики пишут слишком общий код или добавляют функционал, который сейчас не нужен в системе. Ревьюеры должны быть очень бдительны на предмет оверинжиниринга и должны убеждать разработчиков решать только те задачи, которые должны быть решены сейчас, и не касаться того, что, может быть и не придется делать в дальнейшем. Будущие проблемы должны решаться по мере поступления, в тот момент когда они принимают конкретные черты и получают ясные требования.
2.4. Тесты
Код, проходящий ревью, должен быть покрыт тестами: требуйте юнит, интеграционных или end-to-end тестов при проведении ревью. В общем случае, тесты должны быть добавлены в тот же CL, что и код. Исключениями являются
экстренные случаи.
Проверяйте правильность тестов — они не тестируют сами себя, а тесты на тесты пишутся крайне редко, обычно их проверяет человек.
Упадут ли тесты, если сломается код? Будут ли они работать правильно, если код изменится? Все ли тесты простые и проверяют то, что нужно проверять? Корректно ли разбиты проверки по методам?
Помните, что тесты — это тоже код, который нужно поддерживать. Не позволяйте тестам быть слишком сложными, просто потому, что они не входят в конечный релизный файл.
2.5. Наименования
Необходимо проверять, что разработчик выбрал подходящие наименования. Хорошее имя должно быть достаточно полным, чтобы понять, за что отвечает компонент, но вместе с тем, оставаться легко читаемым и не быть слишком длинным.
2.6. Комментарии
Насколько понятны и необходимы комментарии, оставленные разработчиком? Написаны ли они на понятном английском? Комментарии полезны, когда объясняют почему данный код существует, но излишни, если рассказывают о том, что делает код, хотя бывают и исключения: например, регулярные выражения и сложные алгоритмы. Если код сам по себе не является наглядным, то необходимо сделать его проще. Однако, чаще всего, комментарии должны пояснять то, чего не может сказать код: те решения, которые стоят за написанным.
Бывает полезным посмотреть комментарии, оставленные до CL: возможно, там есть «TODO», которое теперь можно убрать или совет про то, как должен быть сделан некоторый функционал.
Важно заметить, что комментарии — это не тоже самое что документация классов, модулей и функций. Документация нужна как раз для того, чтобы описать что делает код и как его использовать.
2.7. Стиль
В Google есть стайл гайды для всех основных и большинства вспомогательных языков программирования, которые мы используем. Убедитесь, что CL соответствует требованиям в соответствующих гайдах.
Если вы делаете замечание, которое касается стиля и этот момент никак не освещается в гайде, то помечайте такое требование как необязательное («Nit:»). CL не должен блокироваться на основании личных стилистических предпочтений.
Разработчик не должен мешать в один CL стилистические правки и исправление функционала. Это делает сложным для понимания, какой именно функционал изменился. Например, если разработчик хочет полностью переписать целый файл, поменяв его функционал и зарефакторив, то требуйте от него двух CL — первый с рефакторингом, а второй уже с функциональными изменениями.
2.8. Документация
Если CL меняет процесс взаимодействия пользователей с кодом, например, то, как теперь работают тесты или проходят релизы, следует также проверять соответствующие изменения в документации, включая README, g3doc (прим. — внутренний инструмент Google для хранения документации) и любые ссылки на сгенерированные документы. Если CL удаляет код, проверьте, что соответствующий раздел в документации также удален. Если документации нет, то запросите ее.
2.9. Каждая строчка
Проверяйте каждую строчку кода. Некоторые данные, такие как сгенерированный код или гигантская структура данных, можно читать по диагонали, но никогда не пролистывайте просто так код, написанный человеком. Конечно, что-то должно быть изучено пристальнее — вы должны сами провести для себя грань что именно и насколько глубоко. Помните, что вы всегда должны понимать что делает код.
Если вам сложно понять что происходит и ревью затягивается, то дайте знать об этом разработчику и, прежде чем продолжать ревью, дождитесь пояснений. В Google работают очень сильные разработчики и вы один из них. Если вы не понимаете, что написано, то, с большой долей вероятности, не поймут и другие. Таким образом, запрашивая пояснения, вы помогаете не только себе, но и вашим коллегам, которые столкнутся с этим кодом в дальнейшем.
Если вы понимаете что делает код, но не чувствуете себя достаточно компетентным для проведения ревью, убедитесь, что среди ревьюеров есть человек с соответствующей квалификацией по данному вопросу. Стоит особенно принципиально относится к этой проблеме, когда дело касается безопасности, доступности, многопоточности, локализации и тд.
2.10. Контекст
Обычно, инструменты для проведения ревью показывают только то, что поменял разработчик и небольшие фрагменты кода около изменений. Но, часто, необходимо полностью просмотреть целый файл чтобы понять контекст: предположим, вы видите, что были добавлены 4 строчки, однако, развернув файл вы понимаете что это строчки в методе из 50 строк и его теперь необходимо рефакторить.
Всегда полезно думать о CL в контексте системы в целом: улучшает ли данный CL код проекта или делает его менее понятным, хуже протестированным и тд?
Не принимайте CL, которые ухудшают состояние кодовой базы. Часто, проекты становятся слишком сложными именно по мере внесения в них мелких изменений, поэтому важно следить за сложностью в каждом отдельно взятом CL.
2.11. Позитивные моменты
Если вы видите в CL хорошо сделанную работу, то скажите об этом разработчику: ревьюеры должны не только обращать внимание на ошибки, но и поощрять за качественно написанный код. С точки зрения менторства намного более важно говорить о том, что сделано хорошо, чем заострять внимание на плохих моментах.
Итоги
Самое важное при проведении ревью:
Вы должны просмотреть каждую строчку кода, брать во внимание контекст, быть уверенным в том, что улучшаете состояние кодовой базы и поощрять удачные решения разработчика.
В следующей части будет рассмотрено, как лучше ориентироваться в CL и с чего начинать ревью.
Код ревью, как внедрить и не испытывать боль
Если вы работаете в продуктовой компании, то жизненный цикл почти каждого продукта будет соответствовать принципу Парето:
20% времени мы пишем новый код.
80% времени поддерживаем старый. Поддержка в себя включает фиксы багов, обновление кодовой базы (переезд на новые библиотеки например).
Во время поддержки мы хотим чтобы все разработчики как можно быстрее вникали в то, что написано. Для этого есть много способов, все они прекрасны и хорошо работают вместе.
Способы иметь хорошо поддерживаемый код:
Все эти способы нужно уметь готовить. Есть тесты, которые только мешают, бесполезная документация и бессмысленный код ревью.
Что же такое код ревью?
Что нам даёт код ревью:
Проверку кода по многим критериям.
Автор не всегда видит неочевидные места в его коде (по разным причинам).
В результате написания и переписывания может быть потеряна композиция.
Автор, находясь в контексте задачи может недостаточно оставить комментариев.
Эти и многие другие проблемы может решить код ревью.
Шаринг знаний о проекте.Не только автор знает, что там пишется в другом проекте или части проекта, а все.
Шаринг знаний о технологиях.Вы можете заметить, что кто-то напилил своих костылей, а похожая проблема в проекте уже решалась. В таком случае ревьюер может подсказать что уже использовалось и избежать ненужного кода
Сотрудники быстрее онбордятся. Новые сотрудники, проводя код ревью, быстрее узнают и понимают традиции команды, а проходя его, имеют возможность исправить ошибки, узнать больше о продукте и меньше испытывать стресс.
Способов поддерживать код «качественным» есть много, но все их нужно правильно готовить. Нужно уметь писать тесты, нужно уметь писать документацию и нужно правильно организовать код ревью.
Советы по организации процесса
Условные обозначения:
— Как делать не нужно
+ Как делать нужно
- Ревьюить только джунов и новых коллег
+ Проводить ревью для всех и всеми
Если ревью проходят не все, то в вашей команде это будет восприниматься как источник недоверия и неравенства. Такую атмосферу следует избегать.
- Обсуждать на код ревью стиль
+ Настроить у себя на проекте инструменты, которые автоматически будут исправлять стиль перед коммитом
В JS это eslint и prettier. Не тратьте время своё и коллег на споры о вкусах. Договоритесь заранее и пропишите правила. В случае разногласий голосуйте.
Не обсуждайте стиль на ревью
- Ревьюер запускает руками билд или проверяет код на баги.
+ Автор сам несет ответственность за поставленный код.
Автор тщательно проверил свой код на работоспособность и залил в удаленный git репозиторий
На сервере CI/CD проверяет тесты, сборку, работоспособность в целом.
Проверка со стороны ревьюера.
- Ревьюер не читает код
+ Обращать внимание на композицию, магические переменные, оптимизацию (там где это имеет смысл)
Обращайте внимание на код ревью.
- Агрессия, негатив, «токсичное» поведение в комментах
+ Старайтесь отмечать не только негативные стороны, но и хвалить за позитивные.
Агрессия, негатив и токсичное поведение в принципе стоит избегать во всех областях жизни. Грубое поведение во время код ревью может привести к:
Стрессу и страху совершить ошибку
Примеры плохих комментов:
Примеры хороших комментов:
«Давай попробуем сделать …» «Может попробуем вынести…»
То же самое касается и ответов на комменты автором. Если автор не согласен, то необходимо объяснять с помощью аргументов, а не фраз в духе «я так делать не буду». Если не получается текстом, иногда проще подойти или сделать короткий созвон, чтобы понять друг друга.
Чаще хвалите код в ревью
- Все комментарии обязательны к исправлению
+ Помечать необязательные комментарии
Пытаться объяснить алгоритмы на словах
Написать пример псевдокода
Особенно актуально, если автор не совсем понимает что от него хочет ревьюер. Не нужно полностью писать реализацию. Напишите небольшой пример псевдокода. Gitlab, github и bitbucket позволяют вставлять в комментарии разметку markdown. Ваш код будет хорошо отформатирован и более понятен, чем длинная цепочка комментов.
- Оставлять больше 50 комментариев
+ Оставлять до 50 комментариев
Если комментариев накапливается очень много, то у вас:
Или не настроены линтеры, и вы не определились со стилем, поэтому комменты о вкусовщине
Или переделать в коде нужно многое
Во втором случае оставьте 1 комментарий с рекомендациями как и что нужно переписать. В таком случае лучше закрыть мердж реквест, переписать код и открыть новый.
Если код будет переписан полностью, отследить изменение по комментариям практически невозможно и они не имеют смысла. Авторы тоже должны это понимать и следить, чтобы всем было комфортно.
- Отправлять огромные фрагменты кода на ревью
+ Дробить большие участки кода на несколько реквестов и вливать постепенно
Задачи бывают разными по объему. Может быть задача исправить 1 строчку кода, а может быть задача отрефакторить весь проект. Во втором случае не отправляйте реквест в котором исправлены 500 файлов и 4000 изменений. Никто в здравом уме не сможет это нормально проверить, и желания такое проверять вы тоже не найдете.
Сделайте ветку feature/epic-name
Изменения делайте в ветке feature/epic-name-implement
Pull реквесты делайте в ветку feature/epic-name. Порционно.
В конце реализации фичи подлейте feature/epic-name в мастер. Да, в этом случае пулл реквест тоже будет огромный, но всё уже будет заранее проверено
Другой вариант как этого избежать: feature flags. Вы вливаете изменения на прод, но не даете им пользоваться. Нормально это реализовано мало у кого. Поэтому с этим подходом нужно быть осторожнее.
Уважайте тех, кто будет проверять ваш PR
- Pull Request висит без ревью трое суток
+ Выработать расписание
Среди аргументов против код ревью вы услышите, что с ним у вас увеличивается время «доставки» фич. Чтобы этого не происходило, выработайте у ревьюеров расписание. Например, новые реквесты проверять до работы и перед уходом домой, а исправленные в перерывах в течении дня (например пока проект собирается или тесты гоняются).
В заключении
В этой статье я хотел рассказать и показать плюсы проведения код ревью. Будучи старшим разработчиком я всегда за то, чтобы мой код проходил code review. Лично для меня это отличный способ “увидеть” свой код чужими глазами. еще до того как ветка отправляется на ревью. Не говоря уже о том, что для команд это недорогой и эффективный способ иметь кодовую базу, с которой можно будет эффективно работать.
Если в вашей команде нет код ревью, то самое время его внедрить .
Ссылки и благодарности
По теме рекомендую почитать:Статья How to Do Code Reviews Like a Human
Спасибо лису и kirjs из @learnInPublic за ревью статьи про ревью.
Как сделать код-ревью быстрее и эффективнее
Как обычно происходит код-ревью? Вы отправляете пул-реквест, получаете обратную связь, вносите исправления, отправляете фиксы на повторный ревью, затем получаете одобрение, и происходит мерж. Звучит просто, но на деле процесс ревью бывает очень трудоемким.
Представьте, что у вас есть пул-реквест с сотнями строк изменений. Ревьюер должен потратить немало времени, чтобы полностью прочитать код и понять предлагаемые изменения. В результате весь процесс от создания пул-реквеста до его утверждения может занять несколько дней — в этом мало приятного и для ревьюера, и для автора изменений. И велики шансы, что в итоге ревьюер все равно что-то упустит. Или проверка может быть слишком поверхностной, а в худшем случае пул-реквест вообще может быть сразу отклонен.
Получается, что чем объемнее пул-реквест, тем меньше пользы будет от его проверки.
Как избежать таких ситуаций? Как сделать пул-реквест проще и понятнее для ревьюера и оптимизировать весь процесс?
Переводим статью нашего бэкенд-разработчика Сергея Жука про то, как устроен процесс код-ревью у команды мобильного приложения Skyeng.
Категории изменений
Давайте представим, что у вас есть задача — реализовать новый функционал в проекте. Пул-реквест, над которым вы работаете, может содержать разные категории изменений. Конечно в нём будет какой-то новый код. Но в ходе работы вы можете заметить, что какой-то код нужно предварительно порефакторить, чтобы он способствовал добавлению нового функционала. Или с этим новым функционалом в коде появилось дублирование, которое вы хотите устранить. Или вы вдруг обнаружили ошибку и хотите ее исправить. Как должен выглядеть окончательный пул-реквест?
Сначала давайте разберемся, какие категории изменений могут происходить с кодом.
Стратегии ревью
Очень важно понимать, что каждая из этих категорий ревьюирутеся по-разному и при их рассмотрении ревьюер должен фокусироваться на разных вещах. Можно сказать, что каждая категория изменений предполагает свою собственную стратегию ревью.
Функциональные изменения. Это самый длительный процесс, потому что он предполагает изменения доменной логики. Ревьюер смотрит, решена ли проблема и проверяет, является ли предложенное решение наиболее подходящим или его можно улучшить.
Структурный рефакторинг. Этот процесс требует гораздо меньше времени, чем функциональные изменения. Но здесь могут возникнуть предложения и разногласия по поводу того, как именно код должен быть организован.
При проверке остальных категорий в 99 % случаев мерж происходит сразу же.
Почему нужно разделять изменения по категориям?
Мы уже обсуждали, что разные категории изменений ревьюируются по-разному. Например, функциональные изменения мы проверяем, отталкиваясь от требований бизнеса, а при структурном рефакторинге — проверяем обратную совместимость. И если мы смешаем несколько категорий, то ревьюеру будет трудно держать в уме одновременно нескольких стратегий ревью. И, скорее всего, ревьюер потратит на пул-реквест больше времени, чем необходимо, и из-за этого может что-то упустить. Более того, если пул-реквест содержит изменения разного рода, при любом исправлении ревьюеру придется пересмотреть все эти категории еще раз. Например, вы смешали структурный рефакторинг и функциональные изменения. Даже если рефакторинг выполнен хорошо, но есть проблема с реализацией функционала, то после исправлений ревьюер должен будет просмотреть весь пул-реквест с самого начала. То есть проверить заново и рефакторинг, и функциональные изменения. Так проверяющий тратит на пул-реквест больше времени. Вместо того, чтобы чтобы сразу смержить отдельный пул-реквест с рефакторингом, ревьюер должен еще раз просмотреть весь код.
Что точно не стоит смешивать
Переименование/удаление класса и его рефакторинг. Здесь мы сталкиваемся с Git, который не всегда правильно понимает такие изменения. Я имею в виду масштабные изменения, когда меняется много строк. Когда вы рефакторите класс, а затем перемещаете его куда-то, Git не воспринимает это как перемещение. Вместо этого Git интерпретирует эти изменения как удаление одного класса и создание другого. Это приводит к куче вопросов во время код-ревью. И автора кода спрашивают, почему он написал этот уродливый код, хотя на самом деле этот код был просто перемещен из одного места в другое с небольшими изменениями.
Любые функциональные изменения + любой рефакторинг. Мы уже обсуждали этот случай выше. Это заставляет ревьюера держать в голове сразу две стратегии рецензирования. Даже если рефакторинг выполнен хорошо, мы не сможем смержить эти изменения, пока функциональные изменения не будут утверждены.
Любые механические изменения + любые изменения, произведенные человеком. Под «механическими изменениями» я подразумеваю любое форматирование, выполненное с помощью IDE или генерации кода. Например, мы применяем новый code style и получаем изменений на 3000 строк. И если мы смешаем эти изменения с какими-либо функциональными или любыми другими изменениями, произведенными человеком, мы заставим ревьюера мысленно классифицировать эти изменения и рассуждать: это изменение, произведенное компьютером, — его можно пропустить, а это изменение, сделанное человеком, — его нужно проверить. Так ревьюер тратит очень много дополнительного времени на проверку.
Пример
Вот пул-реквест с функцией метода, который аккуратно закрывает клиентское соединение с Memcached:
Даже если пул-реквест небольшой и содержит всего сто строк кода, его все равно сложно проверять. Как видно по коммитам, он содержит различные категории изменений:
В результате ревьюер должен просмотреть весь код и
1. Рефакторинг: извлечение класса
Здесь всего два файла. Ревьюер должен проверить только новый дизайн. Если все в порядке — мержим.
2. Следующим шаг — тоже рефакторинг, мы просто перемещаем два класса в новое пространство имен
Такой пул-реквест довольно просто проверять, он может быть смержен сразу.
3. Удаление лишних блоков док-блоков
Здесь ничего интересного. Мержим.
4. Сам функционал
И теперь пул-реквест с функциональными изменениями содержит только нужный код. Так ваш ревьюер может сосредоточиться только на этой задаче. Пул-реквест небольшой и его легко проверить.
Заключение
Не создавайте огромных пул-реквестов со смешанными категориями изменений.
Чем больше пул-реквест, тем труднее ревьюеру понять предложенные вами изменения. Скорее всего, огромный пул-реквест с сотнями строк кода будет отклонен. Вместо этого разбейте его на маленькие логические части. Если ваш рефакторинг в порядке, но функциональные изменения содержат ошибки, то рефакторинг можно спокойно смержить, и таким образом вы и ваш ревьюер сможете сосредоточиться на функционале, не просматривая весь код с самого начала.
И всегда выполняйте следующие шаги до отправки пул-реквеста:
От редакции: Сергей много пишет интересного про программирование и PHP, а мы иногда что-то переводим: сервер потокового видео, рендеринг HTML файлов. Смело задавайте ему вопросы в комментариях к этой статье — он ответит!
Ну а также напоминаем, что у нас всегда много интересных вакансий для разработчиков!