SavePearlHarbor

Процесс ревью кода в HH.RU

—

от автора

admin
Мне на глаза попался документ с правилами и рекомендациями по процессу ревью кода внутри компании. Я решил, что такой полезной информацией надо поделиться с внешним миром. С благословения автора я публикую работу


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

Общие положения

  • В HH.RU ревью проходит в формате пулл-реквестов на Гитхабе.
  • Ревью и пулл-реквесты обязательны, даже если в рамках задачи меняется одна строчка или один символ в коде.
  • У каждой задачи должен быть, как минимум, один ответственный ревьюер, он указывается в задаче в качестве ревьюера и несёт ответственность за код, как и автор. Не запрещено и приветствуется участие в ревью других людей.
  • Ответственность провести задачу через ревью лежит на авторе задачи.
  • Любые изменения после ревью, например, правки во время тестирования, точно так же должны быть проревьюены.

Цели ревью

  • Распространение опыта и знаний между разработчиками.
  • Поддержка принципа — изменения не должны ухудшать уже существующий код, они должны его улучшать.
  • Исправление ошибок в логике и ошибок связанных с безопасностью, корректную обработку исключений.
  • Улучшение качества кода, maintainability и архитектуры проекта.
  • Улучшение читабельности кода.
  • Улучшение покрытия тестами и тестирование на нужном уровне.
  • Улучшение документации.
  • Соответствие изменений требованиям в задаче.
  • Предотвращение появления технического долга или технического налога.
  • Продуманный дизайн API, где API — это любой модуль с внешним интерфейсом. Например, что ресурс в сервисе имеет продуманный URL, удобный формат JSON, корректные коды ответов в разных случаях и так далее.
  • Корректная история коммитов в Гите, с отражающими суть сообщениями, без временных коммитов.

Подготовка к ревью

  • Перед публикацией пулл-реквеста 2-3 раза прочитайте свой код: с большой вероятностью вы сами найдёте там ошибки, что сэкономит время ревьюеру. Проверьте, что нет ошибок, которые могут быть выявлены автоматически — ошибки в стиле кода, упавшие тесты. Удостоверьтесь, что в коде нет временных комментариев и отладочного кода, а также проверьте соответствие вашего кода принятым стайл-гайдам.
  • Если создаёте пулл-реквест в процессе работы над задачей, пропишите в заголовке пометку «[WIP]» и поставьте метку «work in progress». Когда работа закончена, уберите метки и проинформируйте об этом отдельным комментарием, чтобы заинтересованные лица узнали, что код можно смотреть.
  • Будьте готовы показать ревьюеру мини-демо по задаче.
  • Отдавайте на ревью небольшие порции кода. 200-300 строк изменений — предел для эффективного ревью.
  • Оформляйте независимые изменения отдельными коммитами.
  • Описывайте коммиты короткими и информативными сообщениями.
  • Рефакторинг делайте отдельным коммитом, так легче увидеть изменения в логике, суть задачи.
  • Форматирование кода делайте отдельным коммитом до основных изменений, или даже отдельным пулл-реквестом.
  • Не коммитьте незначащие изменения. Например, добавление переносов строк в файле, в котором вы больше ничего не меняли.
  • В заголовке пулл-реквеста коротко и информативно опишите суть изменений.
  • Опишите в пулл-реквесте проблему и решение. Опишите альтернативные варианты решения и почему выбрано текущее решение. Если изменения касаются интерфейса, приложите скриншоты «до» и «после».
  • В пулл-реквесте дайте ссылку на задачу, а в задаче ссылку на пулл-реквест.
  • Иногда полезно обсудить выбранное решение с ревьюером до написания кода. Это поможет найти оптимальный вариант решения задачи и упростит процесс ревью.

Выбор ревьюера

  • Отдавайте задачу на ревью специалисту из соответствующей области.
  • Если с каким-то кодом работает несколько команд, то полезно для распространения знаний выбирать ревьювера из другой команды.
  • Сотрудник на испытательном сроке не может быть ответственным ревьюером, но может участвовать в процессе ревью.
  • Не отдавайте все ваши задачи на ревью одним и тем же людям — просите ревью у разных людей.
  • Если в задаче затронут нетривиальный участок кода, в котором разбирается определённый человек, покажите изменения ему, независимо от того, кто ответственный ревьюер. Также помните, что у каждого репозитория есть меинтейнеры, они больше других знают об этом коде и курируют разработку.
  • В остальном ревьюер выбирается произвольно, по обоюдному согласию сторон. Для помощи в выборе используйте рекомендации на Гитхабе, основанные на «блейме» затронутого кода.
  • После выбора ревьюера укажите его в качестве Assignee в пулл-реквесте. Список всех пулл-реквестов, которые на вас назначены.

Процесс ревью, общие положения

Относится как к автору кода, так и к ревьюеру.

  • Весь код общий, нет таких понятий как «мой код» или «твой код». Это значит, что за любую строчку кода отвечает каждый разработчик, а не только тот, кто эту строчку написал.
  • Создавайте атмосферу взаимопонимания, будьте вежливы и дружелюбны, цените и уважайте друг друга, не навязывайте своё мнение. Прислушивайтесь к мнению оппонента и не стойте на своём из принципа. Не превращайте ревью в отрицательный опыт для коллег.
  • Задавайте вопросы, если что-то не понятно. Аргументируйте и конкретизируйте вопросы и ответы. Автору кода может быть не очевидно, что подразумевается под вопросом «почему так?», а ревьюверу непонятна аргументация ответа «не согласен».
  • Не растягивайте процесс ревью, экономьте время, оперативно реагируйте на комментарии, обсуждайте устно, не провоцируйте длинные дискуссии и не разводите «холиваров». Если вы не можете быстро прийти к консенсусу обратитесь за помощью к коллегам, к меинтейнеру или руководителю функционального направления.
  • Если задача не на пару строк, потратьте с ревьюером 10-15 минут на совместный просмотр кода, полезно сделать это даже до создания пулл-реквеста. Обсудите суть задачи и решения, посмотрите как было и стало в работающем окружении. Это поможет ревьюеру лучше погрузиться в контекст задачи. Большинство комментариев останутся ненаписанными, что сэкономит всем время.
  • После устного обсуждения всегда описывайте принятое решение и доводы в пользу него в пулл-реквесте. Ответственность за описание лежит на авторе кода.
  • Если в коде встречаются однотипные ошибки, ревьюеру следует акцентировать на этом внимание автора кода в первом или втором комментарии, не комментируя каждое вхождение, а автору анализировать код на предмет однотипных ошибок перед публикацией исправления.
  • Хвалите за удачные решения автора и предложения ревьювера.
  • Оставляйте комментарии к изменениям в самом пулл-реквесте, а не к отдельным коммитам. Таким образом, вся переписка будет в одном месте, в том числе после ребейза.
  • Отвечайте на комментарии в тех же ветках обсуждений, где они начаты. Если комментарий относится ко всему пулл-реквесту или в одной ветке несколько комментариев, цитируйте комментируемое сообщение. Чтобы из письма перейти на устаревшую ветку обсуждения вместо ссылки «view it on GitHub» используйте ссылку «in path/to/file».
  • Если в процессе ревью приняты какие-либо кардинальные решения с точки зрения стандартов кодирования или иных оформленных договорённостей, на автора кода возлагается обязанность записать эти решения в соответствующих документах.
  • Ревью не только про код, а про задачу целиком. Не относитесь к требованиям в задаче или макетам как к истине в последней инстанции — все совершают ошибки и никто не может учесть все нюансы. Свежий взгляд и дельные предложения только пойдут продукту на пользу.

Процесс ревью со стороны автора кода

  • Задача не завершена, пока не прошла ревью, тестирование и выпуск на «прод».
  • Отвечайте на все комментарии ревьюера, не оставляйте комментарии без ответа, независимо от того приняли вы их или нет.
  • Задумывайтесь над вопросами, которые возникают или могут возникнуть во время ревью. Возможно, участку кода не хватает комментария или код лучше написать более явно.
  • Не исправляйте существующие коммиты «амендом» (git commit —amend), вместо этого вносите изменения отдельными коммитами, по одному на каждое исправление. Исправление существующих коммитов сильно затрудняет процесс ревью, так как приходится смотреть весь код заново.
  • После добавления нового коммита к пулл-реквесту, напишите отдельным комментарием сводку об изменениях, чтобы заинтересованные лица были в курсе. Это касается как исправлений во время ревью, так и исправлений во время тестирования.
  • После ревью, перед тем, как отправить задачу в тестирование, перепишите историю коммитов (git rebase —interactive), внеся в отдельные коммиты соответствующие исправления и удалив временные коммиты. Обратите внимание на опцию —autosquash для упрощения процесса.

Процесс ревью со стороны ревьюера

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

Использованные материалы и ссылки по теме

P. S. Делитесь в комментариях своими правилами, рекомендациями и полезными юзкейсами по процессу ревью


ссылка на оригинал статьи https://habr.com/post/422399/

Комментарии

Добавить комментарий

Ваш адрес email не будет опубликован. Обязательные поля помечены *