При проведении code review через github какие файлы нужно проверять

Обновлено: 07.07.2024

Что будет, если случайно или намеренно забыть про code review, можно ли перепоручить кому-то данную фазу работы и есть ли универсальный эффективный алгоритм для проверки кода? На эти вопросы точно знают ответ Илья Гумбар и Владислав Наруцкий, Software Engineers iTechArt.

Илья Гумбар

Software Engineer iTechArt

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

Владислав Наруцкий

Software Engineer iTechArt

Благодаря code review отслеживает, чтобы код передавал намерения разработчика и был понятен даже человеку, который не погружен в данную проблему.

Насколько важен code review? Что будет, если его пропустить?

ИЛЬЯ ГУМБАР: Code review безусловно важен при командной работе. В случае успешно налаженных процессов code review

  • помогает контролировать технический долг проекта
  • улучшает читаемость и поддерживаемость кода
  • снижает количество дефектов/багов
  • способствует knowledge sharing

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

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

Также у code-review есть ещё одна не менее важная цель – это поддержание кода и архитектуры в «чистоте». Я не буду сейчас подробно останавливаться на определении «чистоты», про это написано уже немало книг (например «Чистый код» и «Чистая архитектура» Р. Мартина), но просто скажу одну, возможно очевидную, вещь – на самом деле программисты работают не с машинами, а с людьми посредством своего кода. Поэтому в течение code review я также слежу за тем, чтобы код передавал намерения разработчика, и был понятен даже человеку, который не погружен в контекст, ведь скорее всего в будущем код придется поддерживать другим людям. А если позже в нем будет непросто разобраться, то это снова повлечет за собой временные издержки.

Насколько code review – трудоёмкий этап? Сколько времени на него требуется?

ИЛЬЯ ГУМБАР: Все зависит от проекта и вносимых изменений. Как правило, большинство команд стараются контролировать размер правок за счет соблюдения некоторых правил таких как

  • PR (pull request) должен содержать изменения которые затрагивают только один use-case.
  • PR не должен содержать одновременно и новую функциональность и рефакторинг и т.д.

Качество проверки обратно пропорционально размеру проверяемых изменений. Т.е. чем больше PR, тем выше шанс что либо упустить. Соответственно, временные затраты на code review контролируются командой и как правило не превышают 30 минут.


ВЛАДИСЛАВ НАРУЦКИЙ: Я бы сказал, что достаточно трудоёмкий, т.к. одной из сложностей является понять, как думает другой человек. Бывает, что ты уже точно знаешь, как бы ты решил этот вопрос, а потом при проверке видишь совсем не то, что ожидал. И тут есть момент, когда важно не поддаться искушению, что любое решение, которое не совпадает с твоим – неправильно. Надо все-таки дать альтернативному мнению шанс и вникнуть в него, возможно оно более оптимальное. Это бывает сделать непросто, т.к. в твоей голове уже сложился образ правильного решения.

Еще одной сложностью, я бы сказал, является сложность решаемой задачи. Когда идёт имплементация какой-либо сложной бизнес-логики, или бизнес-флоу, случается, что затронуты множество файлов в разных частях приложения. Вникнуть в такое решение бывает сложнее, и оно, в свою очередь, потребует больше времени для code review. Следует также обращать внимание на то, были ли затронутые какие-либо переиспользуемые вещи. Если да – то надо оценить, а могло ли данное изменение что-либо где-либо сломать, и т.д.

Если постараться дать конкретную временную оценку - то я бы сказал, что code-review может занимать от 5-10 минут до 1-2 часов в зависимости от сложности и объёма задачи.

На кого можно делегировать code review?

ИЛЬЯ ГУМБАР: В различных командах могут применяться различные стратегии. Качество и количество ревьюверов также может меняться в зависимости от области, куда вносятся изменения. Как пример, изменения одобряют как минимум двое: Tech Lead и разработчик, который так или иначе был вовлечен в затрагиваемую функциональность.

С точки зрения knowledge sharing, чем больше людей просмотрит PR, тем лучше. Так что в случае, когда PR привносит изменения в ядро проекта, либо когда расширяется библиотека общих компонентов, будет не лишним оповестить команду (либо ее часть).

ВЛАДИСЛАВ НАРУЦКИЙ: Я думаю, что делегировать code review можно любому достаточно опытному коллеге из группы, который хорошо ориентируется в предметной области и бизнес-требованиях продукта, а также в архитектуре проекта/системы. Также данный ревьювер должен уметь анализировать различные подходы и решения, уметь оценить их плюсы и минусы, чтобы по итогу выбрать максимально простое и оптимальное из всех возможных.

Автор кода VS ревьюер. Какие обязанности есть у каждой стороны?

ВЛАДИСЛАВ НАРУЦКИЙ: Я думаю, что обязанности сторон примерно следующие. Автор кода должен найти максимально простое и оптимальное решение, при этом сохранив код в «чистоте» (о которой уже было сказано выше). А ревьювер, в свою очередь, должен это подтвердить. Если же это не так, то ревьюверу следует дать конструктивный фидбек и предложить альтернативы автору. Также на ревьювера ложится обязанность проанализировать, действительно ли все были случаи покрыты. То есть не только убедиться, что код, который был написан корректен, но и также убедиться, что никакой другой код не должен быть написан.

ИЛЬЯ ГУМБАР: Обе стороны должны быть дружелюбны, вежливы и уважительны друг к другу в общении. Также нужно помнить, что цель у двух сторон одна – улучшить какой-либо аспект системы.

  • Внимательно проанализировать предлагаемые изменения по следующим критериям: архитектура, функциональность, сложность, тесты, code style, best practices и.д.
  • На заключительном этапе ревьювер должен запустить изменения у себя на машине и проверить их соответствие поставленной задаче.
  • Оставить список замечаний на основе анализа изменений. Необходимо, чтобы правки реально способствовали улучшению качества кода по какому-либо аспекту.

В случае наличия налаженных процессов конфликтные ситуации исключены, а в случае их наличия регламентированы способы их устранения.

С помощью каких сервисов можно делать code review? Выдели их основные достоинства и недостатки

ИЛЬЯ ГУМБАР: Как правило, большинство сервисов управления проектами имеют те или иные встроенные возможности для code review. Мне не довелось проводить сравнительный анализ каждой, но могу выделить функциональность, которая так или иначе должна быть реализована для максимального удобства:

  1. Связь с бэклогом
  2. Связь системой CI/CD
  3. Связь с системой документации /wiki
  4. Возможность оставлять замечания к строчке, блоку, либо ко всему PR
  5. Возможность создавать задачи на основе замечаний и связывать их с PR
  6. Возможность предлагать изменения (в виде diff)
  7. Возможность отслеживать и контролировать статус правок
  8. Возможность автоматически назначать ревьювера(ов), используя гибкую систему правил
  9. Поддержка расширенного синтаксиса для оформлени PR и замечаний (ссылки, картинки, чек-листы и т.д.)

Примеры: GitHub, Azure DevOps, Atlasian tools

ВЛАДИСЛАВ НАРУЦКИЙ: Наиболее часто приходилось делать code review через BitBucket и GitHub. Поэтому, сравнивая эти два сервиса между собой, могу отметить следующее:

  • BitBucket более удобен тем, что отображает структуру файлов и папок, что упрощает навигацию по сравнению с GitHub.
  • В GitHub же более удобно организовано комментирование, а также подсветка синтаксиса, что упрощает непосредственно сам процесс code review.

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

ВЛАДИСЛАВ НАРУЦКИЙ: Некоторые из принципов Google присущи моей работе. Например, стараюсь избегать личных предпочтений касательно некоторых вещей и всегда с ребятами обсуждаю детали, которые мне непонятны, чтобы вникнуть в поток размышлений автора кода. Также слежу за соблюдением code styles, и придерживаюсь их сам.

Твой личный алгоритм код ревью?

ВЛАДИСЛАВ НАРУЦКИЙ: Мой личный алгоритм code review состоит примерно из следующих шагов:

Перевод статьи «Code Review — The Ultimate Guide».

Руководство по ревью кода

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

Будем считать, что само понятие «ревью кода» для вас не ново. Если нет, с ним можно ознакомиться на вики.

Давайте быстро повторим, что нам вообще дает просмотр кода:

  • Уменьшение количества багов в коде.
  • Дополнительная проверка удовлетворения всех требований.
  • Эффективный способ учиться у коллег и знакомиться с кодовой базой.
  • Поддержка определенного стиля кода внутри команды.
  • Сплоченность команды: ревью кода подталкивают разработчиков к общению друг с другом на тему лучших подходов и стандартов кода.
  • Улучшение качества кода под влиянием коллег.

Однако ревью кода может стать одной из самых сложных и затратных по времени частей процесса разработки программы.

Если процесс ревью кода не спланирован правильно, «затраты» на его проведение могут превысить выгоды.

Вот почему так важно создать в вашей команде хорошую структуру процесса ревью кода.

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

Определяем необходимые условия для создания пул-реквеста

  1. Убедитесь, что ваш код успешно компилируется.
  2. Прочтите свой код, добавьте аннотации.
  3. Соберите и запустите тесты, соответствующие области вашего кода.
  4. Весь код кодовой базы должен быть протестирован.
  5. Сделайте ссылку на относящиеся к делу тикеты в вашем инструменте для менеджмента задач (например, JIRA).
  6. Не назначайте ревьюера, пока не сделаете все вышеперечисленное.

Определяем обязанности автора кода

Чем в лучшем виде вы передадите свой код ревьюеру, тем меньше рисков это повлечет в долгосрочной перспективе. Вам могут помочь следующие рекомендации:

Просмотр кода

Определяем обязанности ревьюера

Поскольку ревьюер является последним звеном в цепочке перед слиянием кода, уменьшение количества ошибок в этом коде по большей части лежит именно на нем. Ревьюер должен:

  1. Ознакомиться с описанием задачи и требованиями.
  2. Убедиться, что он полностью понимает представленный ему код.
  3. Оценить все архитектурные компромиссы.
  4. Разделить свои комментарии на три группы: критически важные, опциональные и позитивные. В первую группу попадают изменения, которые разработчик обязательно должен внести. В последней будут комментарии, которые дадут разработчику понять, что вы оценили красоту отдельных частей его кода.

Также стоит избегать большого количества комментариев. Вместо этого можно использовать Github review (см. пример ниже).

Если у вас несколько замечаний, не нужно писать комментарий по каждому в отдельности. Используйте опцию ревью на Github и пошлите уведомление разработчику (PR owner), когда завершите.

Наконец, я считаю, что следующие вопросы очень помогают улучшить и облегчить процесс ревью кода:

  • Возникают ли у меня трудности с пониманием этого кода?
  • Есть ли в коде какие-то сложности, которые можно уменьшить путем рефакторинга?
  • Хорошо ли организован код, продумана ли структура пакета?
  • Являются ли имена классов интуитивно понятными, очевидно ли, что эти классы делают?
  • Есть ли заметно большие классы?
  • Есть ли какие-то особенно длинные методы?
  • Все ли имена методов кажутся понятными и интуитивными?
  • Хорошо ли документирован код?
  • Хорошо ли он протестирован?
  • Можно ли сделать этот код более эффективным?
  • Соответствует ли этот код нашим командным стандартам стиля?

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

Рано или поздно для каждого программиста настаёт время отвлечься от собственного кода и оценить чужой. Осознав неизбежность этой работы, вам нужно будет решить, как цензурно выразить всё, что вы думаете о рецензируемом коде. Мы расскажем, как с этой задачей справляются в Google.

Стандарты

Главная цель code review в Google — постоянно совершенствовать кодовую базу. Соответственно, если вы делаете обзор на код, являющийся частью большого проекта, — подумайте в первую очередь не о сиюминутных решениях, а о том, как это повлияет на весь проект в перспективе. Есть два аспекта, которые вам придётся отбалансировать.

Дилемма обозревателя

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

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

Главное правило

Запомните, именно вам предстоит найти баланс, позволив кодерам развиваться, и в то же время не пожертвовав качеством кода.

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

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

Наставничество

Оставляйте комментарии, делитесь своим опытом, в перспективе это улучшит код, который достанется вам от этого разработчика в будущем. Однако следует разделить указания, обязательные к выполнению и общие рекомендации. В Google советуют использовать для последних префикс «Nit:» (от слова « nitpicking», придирка) . Не обязательно обозначать рекомендации именно так, это просто общеупотребительный способ.

Принципы

  • Факты и данные важнее личного мнения и персональных предпочтений.
  • Стиль кода должен соответствовать принятому в вашей команде. Если какой-то момент не оговорён — оставляйте его на усмотрение кодера.
  • Архитектура кода должна соответствовать принципам, лежащим в основе проекта. Если есть несколько способов решить задачу — выбирайте тот, который соответствует этим принципам. Если есть несколько равно эффективных вариантов — оставьте выбор за автором кода.
  • Можно попросить автора согласовать стиль кода с текущей кодовой базой, если это не ухудшит общего качества.

Разрешение конфликтов

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

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

Что нужно проверить в рецензируемом коде

  • Общая структура. Как код вписывается в ваш проект.
  • Функциональность. Способен ли код полностью удовлетворить поставленные задачи.
  • Удобство. Интуитивность UI и соответствие его общему стилю.
  • Многопоточность. Рецензируемый код не должен конфликтовать с другими элементами кодовой базы при многопоточном выполнении. То же касается и внутренних конфликтов кода.
  • Простота. Код не должен быть слишком громоздким. Максимально упрощаем, но не в ущерб качеству и функциональности.
  • Перспектива масштабирования. Возможно, в коде могут быть реализованы некоторые возможности, востребованные в будущем, сообщите об этом разработчику.
  • Наличие тестов (модульные, интеграционные и так далее). Внимательно изучите их структуру.
  • Преемственность разработчиков. Все переменные, поля, функции, вообще все объекты и элементы в коде должны иметь ясные, однозначные имена. Комментарии к коду ясно и чётко объясняют, зачем нужен каждый элемент. Обратите внимание, вопрос «зачем это» важнее вопроса «что это».
  • Соответствие стандартам. Код должен соответствовать стандарту стиля и быть должным образом документирован.

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

Последовательность действий при рецензировании кода

  1. Оцените, имеют ли смысл предложенные изменения. Возможно, кодер пытается улучшить фичу, от которой ваш проект собирается избавиться. Направьте его усилия в нужное русло.
  2. Изучите главную часть рецензируемого кода. Если затрудняетесь выделить эту главную часть, спросите разработчика. Найдя какие-то недостатки, немедленно сообщите автору кода. Переходить к изучению деталей кода до исправления основных недочётов нет смысла, вполне вероятно, что впоследствии эти детали значительно изменятся.
  3. Изучите остальную часть кода. Ориентируйтесь на логическую последовательность действий и проверьте каждый файл, возможно, сначала имеет смысл разобрать модульные тесты, чтобы иметь представление о том, какие именно изменения планирует внести разработчик.

Скорость подготовки code review

Чем чреваты медленные code review?

  • Уменьшается скорость работы всей команды. Как цепь не сильнее самого слабого звена, так и команда не быстрее самого медленного её участника.
  • В Google считают, что обозреватель, появляющийся раз в несколько дней с ворохом комментариев, вызывает раздражение и жалобы разработчиков. В компании считают, что гораздо эффективнее незамедлительно реагировать на каждый апдейт от кодера, предлагая пусть и небольшие, но положительно влияющие на качество кодовой базы изменения.
  • Чем сильнее вы затягиваете с code review — тем сильнее желание добавить код в базу «как есть», а это чревато ухудшением общего качества.

Как быстро нужно делать code review?

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

Единственное исключение из этого правила — если вы сконцентрированы на другой задаче. Переключение займёт слишком много времени.

Постепенно вы сможете увеличить скорость работы. Но не делайте это в ущерб качеству.

Работа с большими объёмами

Экстренные ситуации

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

Как писать комментарии во время рецензирования

  • Будьте благожелательны. Вы с разработчиком на одной стороне баррикад. Рецензируйте код, а не кодера.
  • Не будьте императивны, поясняйте свои рекомендации и указания. Это поможет разработчику лучше понять и выполнить их.
  • Объясните, чего вы хотите, но не делайте работу за кодера. Как правило, он всё же может сделать её лучше. Помните, разработчику нужно дать возможность развиваться. Опять же, не в ущерб качеству. Ищите баланс.
  • Предложите разработчику попробовать упростить код, а если это невозможно — добавить подробные комментарии.

Работа с возражениями

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

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

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


После проведения сотни code rewiew, лично возглавив R&D (Research & Development) команду и спровоцировав несколько непреднамеренных ошибок, я решил поделиться своими выводами о том, как правильно выстроить процесс проведения code rewiew.

Давайте сформулируем несколько простых причин, почему вы вообще должны прибегать к code rewiew:

  1. Может помочь уменьшить количество ошибок в коде.
  2. Поможет убедиться, что все требования по оформлению кода соблюдены.
  3. Это эффективный способ обучиться чему-нибудь новому у своих коллег и ознакомиться с данной базой кода.
  4. Помогает придерживаться единого стандарта по оформлению кода всей вашей команде.
  5. Сплачивает команду — code rewiew стимулирует разработчиков общаться друг с другом и вместе придумывать решения проблем.
  6. Улучшает общее качество кода.

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

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

Если процесс проведения code rewiew был неправильно спланирован, затраты на его проведение будут превышать конечную ценность.

Вот почему крайне важно правильно организовать и выстроить четко определенный процесс проведения code rewiew в вашей команде.

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

Необходимые предварительные условия для создания pull-запросов.

Я пришел к выводу, что следующие условия чрезвычайно помогают в снижении количества разногласий:

  1. Убедитесь, что код успешно компилируется.
  2. Прочитайте и прокомментируйте свой код.
  3. Создайте и запустите тест, который проверит на ошибки область видимости вашего кода.
  4. Весь код в кодовой базе должен быть протестирован.
  5. Соедините релевантные задачи/вопросы в своей системе для отслеживания ошибок и управления проектом (например, в Jira) cо своим pull-запросом.
  6. Не “приглашайте” эксперта, пока не завершите все вышеизложенные пункты.

Обязанности того, чей код проверяют

Обязанности рецензента.

Кроме того, избегайте чересчур большого количества замечаний и используйте Github review (см.пример ниже)


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

И наконец, я обнаружил, что, если вы будете задавать следующие вопросы — это поможет вам упростить и улучшить процесс code review:

Читайте также: