- Отсутствует стандарт оформления кода
- Не используются автоматизированные проверки
- Программисты не выполняют самостоятельный анализ своего кода
- Огромные пул-реквесты
- Расплывчатые пул-реквесты
- Отсутствуют дедлайны для код-ревью
Отсутствует стандарт оформления кода
Каждая команда должна принять стандарт оформления кода (стандарт программирования, руководство по стилю), с которыми все согласны. В него входят соглашения об именовании, структуре папок, форматировании кода и список лучших практик, таких как модульное тестирование, валидация и т. д.
Отсутствие чёткого стандарта и соглашений заставляет каждого разработчика писать код так, как ему нравится, что приводит к спорам на код-ревью. Если вы видите много комментариев по поводу форматирования, соглашений об именовании и прочее, то пришло время поговорить об общем стандарте.
Ваша команда может либо разработать собственный набор рекомендаций, либо начать с известных рекомендаций от других компаний. Вот пример от Google.
Не используются автоматизированные проверки
После принятия стандартов оформления кода изучите инструменты для проверки соответствия этим стандартам. Почти для всех языков есть инструменты для форматирования кода, линтеры и другие утилиты, которые вы можете поставить в хуки перед коммитом и прописать в системах CI/CD.
Эти инструменты проверяют код на соответствие стандартам оформления и уведомляют автора о нарушениях. Автор получает возможность исправить проблемы перед отправкой пул-реквеста, что значительно снижает шум. Чем больше проверок проходит автоматически, тем больше времени остаётся у рецензента, чтобы сосредоточиться на более серьёзных проблемах, таких как огрехи архитектуры, пробелы в реализации и т. д.
Программисты не выполняют самостоятельный анализ своего кода
Прежде чем обращаться к коллегам за код-ревью, автор должен сам просмотреть собственные изменения. Это как проверка текста письма на опечатки и ошибки перед отправкой адресату.
На практике проверка собственного кода является сложной задачей, поскольку вы склонны подсознательно пропускать недостатки. Вот несколько способов улучшить качество самостоятельного анализа:
- Оформить пул-реквест без назначения рецензентов и вернуться к нему через пару часов, чтобы свежим взглядом взгянуть на свои правки.
- Подавлять свою естественную склонность пролистывать изменения, а вместо этого намеренно просматривать их строчку за строчкой.
- Следовать контрольному списку вопросов: соответствует ли код стандарту оформления? удовлетворяются ли все бизнес-требования? есть ли тесты для всех возможных вариантов использования? и т. д.
Огромные пул-реквесты
Количество комментариев к пул-реквесту обратно пропорционально количеству содержащихся в нем изменений. То есть большой PR > мало комментариев, маленький PR > много комментариев.
Дело в том, что рецензенты не внимательно изучают большой пул-реквест, а вместо этого склонны пролистать его, чтобы быстрее завершить работу. Это противоречит цели код-ревью. Иногда происходит обратное, когда автор много дней не получает никаких комментариев, потому что рецензент боится начать обзор слишком большого PR.
Разбейте пул-реквест на более мелкие куски, которые имеют смысл отдельно друг от друга. Так вы поможете рассмотреть их должным образом и быстро.
Расплывчатые пул-реквесты
Часто вам предлагают пул-реквест с расплывчатым описанием или вообще без оного. Рецензент должен понять смысл изменений, пытаясь вспомнить задачу, которую обсуждали где-то на собрании или в баг-трекере. Поэтому PR лучше снабжать сопроводительной информацией:
- Какие изменения в нём содержатся
- С каких файлов начать
- Ссылка на задачу в баг-трекере
- Скриншоты, если это какое-то визуальное изменение
Эта информация обеспечит лучший контекст для рецензента и тем самым ускорит код-ревью.
Отсутствуют дедлайны для код-ревью
Один из способов растянуть код-ревью навечно — не поставить никаких дедлайнов для рецензента. Определите разумные сроки, например:
- Код-ревью должен быть закончен за 48 часов с момента отправки пул-реквеста.
- Для хотфиксов срок 30 минут.
Спасибо за чтение! :)
Maksclub
Огромныей пулл-реквест — как кашель, побочный эффект, а не самостоятельная проблема.
Почему он большой? Потому что или задача большая, или задач несколько, но из-за долгого процесса выкладки копятся они (задачи/пуллреквесты), а для текущего небольшого PR нужны изменения из другого, вот и принимается решение в одном и написать, иначе потом при рефакторинге корневого все превращается в какую-то бойню. Еще и конфликты с другими.
Рецептом вижу — мелкие PR с минимальным инкрементом, которые могут долетать до прода «сегодня». А это довольно интересный и часто сложный вопрос для организации разработки/QA.
…
Вообще кайф, когда мелкие PR доходят до прода. Дофамин и вот это все ощущение удовлетворенности
Eldhenn
Пример из жизни. Предприятие занимается одним видом деятельности. Торгует сникерсами. Торгует сникерсами 15 лет, и делает это довольно успешно. И тут решили торговать не только сникерсами, но и мороженым. А ещё через пять лет — рыбой. И ещё через пару лет решили запустить собственный супермаркет, все как у больших.
Как тут бить на мелкие икременты, которые могут долетать до прода?
Пример жизненный, сроки реальные, чуть-чуть изменена отрасль.
Dolios
Я вот тоже слабо понимаю, как эту сову на глобус в реальном мире натягивать.
hippoage
PR не означает, что новая функциональность доступна. Можно, например, включать через feature flags.
PR не означает, что сразу пойдет в релиз. Например, есть один большой PR — разбили на небольшие, они прошли все процессы в рамках одного релиза.
А про мелкие инкременты — это как раз реальность. Как в софте, так и в реальном мире. Например, сначала провели маркетинговый анализ, потом нашли поставщиков, заключили договора, добавили товар в кассу, изменили инструкции работы персонала, заказали новые маркетинговые материалы, закупили морозильные лари для мороженного — много-много разных этапов как в оффлайн, так и в софте. Понятно, что ценность начинается с момента начала продаж мороженного, но как до, так и после начала есть множество этапов, которые имеет смысл постепенно поставлять.
Хорошая аналогия малых PR в магазине: выложить новые рекламные таблички, переставить товар и т.п.: их много, они нужны практически каждый день, они немного улучшают продажи. Это не отменяет и большие, но они обычно так же вполне реально постепенно добавляются, хоть и не (особо) заметно для пользователя, пока выключены.
Eldhenn
> сначала провели маркетинговый анализ, потом нашли поставщиков, заключили договора, добавили товар в кассу, изменили инструкции работы персонала, заказали новые маркетинговые материалы, закупили морозильные лари для мороженного
А потом до IT доводят, что через две недели запускается новая продукция, и срочно нужна поддержка в информационной системе.
hippoage
И что? В условиях аврала наоборот нужно достаточно часто показывать как получается, такое за 1 итерацию не сдается (и не заработает за 1 итерацию нормально). По идее накладных расходов на это не должно быть (если все пайплайны уже готовы до аврала).