код ревьюер что это
Код ревью, как внедрить и не испытывать боль
Если вы работаете в продуктовой компании, то жизненный цикл почти каждого продукта будет соответствовать принципу Парето:
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 за ревью статьи про ревью.
Как проводить Code Review по версии Google
Вопросы код-ревью меня интересуют очень давно. Много раз возникали те или иные проблемы то с качеством кода, то с климатом в коллективе. И действительно, code review — это если не единственное, то одно из самых главных мест для возникновения конфликтов в коллективе разработчиков.
И вот недавно при подготовке к очередному выпуску подкаста «Цинковый прод» я узнаю, что Google опубликовал свод правил по проведению Code Review, битком набитый ценными мыслями. Весь материал довольно объемный и не влезет в одну статью, поэтому я постараюсь выделить наиболее интересные (мне) мысли.
Термины, используемые в оригинальной статье:
CL — changelist. Но обычно это называют Merge Request или Pull Request, поэтому в статье я буду использовать сокращение MR
LGTM — Looks Good to Me. Короче, когда жмут кнопку «approve». Я буду использовать термин «апрув», как более понятный населению.
Идеальность MR
На практике можно встретить различные крайности при рассмотрении MR. Кто-то всей командой задалбывает замечаниями, пока всё до мелочей не будет исправлено, кто-то смотрит примерно логику и жмёт «апрув».
В Гугловском документе написана интересная мысль. MR не должен быть идеальным, но он должен улучшать кодовую базу. Т.е с каждым привносимым изменением код должен становиться лучше и лучше. И если MR добавляет много хорошего, то не надо придираться к мелочам; выгоднее, чтобы это улучшение попало в код побыстрее.
Никакой MR не должен делать код хуже. Единственное исключение — это если MR является срочным фиксом чего-нибудь.
Свобода делать замечания
Несмотря на то, что нельзя задалбывать мелочами, тем не менее можно свободно эти мелочи писать хоть к каждой строчке. Противоречие с предыдущим пунктом решается просто: мелочи и придирки ревьювер помечает префиксом «nit:» от англ. nitpick (придирка). Такие замечания фиксить необязательно, однако автор MR может захотеть что-то исправить или, даже если нет, учесть на будущее некоторые моменты.
Факты важнее персональных предпочтений
Почти всегда стандартные принципы построения ПО (software design), базирующиеся на лучших практиках, лучше, чем хитровымученные велосипеды. Поэтому предпочтение нужно отдавать им.
Если можно применить несколько стандартных подходов, то это на усмотрение автора.
Прямая цитата для лучшего понимания:
Aspects of software design are almost never a pure style issue or just a personal preference. They are based on underlying principles and should be weighed on those principles, not simply by personal opinion. Sometimes there are a few valid options. If the author can demonstrate (either through data or based on solid engineering principles) that several approaches are equally valid, then the reviewer should accept the preference of the author. Otherwise the choice is dictated by standard principles of software design.
Если ревьювер и автор MR не согласны друг с другом
Сначала идет попытка достигнуть консенсуса в комментариях к MR. Если это не получается, то личное обсуждение. Если все равно не пришли к единому мнению, то привлекать членов команды. Но главное, MR не должен застревать надолго из-за несогласия двух человек.
Скорость проверки MR
Скорость экстремально важна. Если раз в несколько дней давать по одному комменту, то автор MR будет жаловаться, что к нему придираются и не дают работать.
Если MR проверяется очень быстро, то любые замечания не будут являться большой проблемой — ведь их фиксы тут проверят, и таск пройдет дальше.
В Гугле советуют не отвлекаться от фокусировки над своей задачи, но если уж отвлеклись, то просмотреть, нет ли чего-то поревьювить. Например, вернулся с обеда — поревьювил. Пришел на работу — поревьювил. И т.д.
Порядок просмотра MR
Сначала надо просмотреть MR в целом. Надо ли это вообще, в правильном ли месте код (или это должно быть в отдельной библиотеке), есть ли какие-то глобальные проблемы.
Т.е. нет смысла смотреть какие-то детали реализации, если код вообще не туда и не для того.
Вообще, порядок просмотра важен, чтобы как можно быстрее выдавать автору фидбек (смотри предыдущий пункт).
Когда посмотрели MR вцелом, надо пройтись по основным файлам, т.е. проверить самое главное (если непонятно, что самое главное, можно спросить у разработчика).
Опять же, о любых проблемах нужно сообщать сразу, даже если вы еще не досмотрели MR и решили вообще досмотреть его позже.
Далее надо выбрать логичную последовательность просмотра оставшихся файлов. Ни один файл не должен быть пропущен.
На что обращать внимание при просмотре
Слишком большие MR
Слишком большие MR нужно просить разбивать на части. Почти всегда это возможно.
Как писать комментарии на код ревью
Если с вашим мнением не согласны
Нужно разобраться детально. Возможно, вы чего-то не понимаете, запросите больше информации. Возможно наоборот. В любом случае, зачастую понимание приходит только после пары раундов объяснений с обеих сторон.
Боязнь расстроить автора MR
Бывает, что разработчик расстраивается, если ревьювер настаивает на каких-то изменениях. Однако, чаще всего разработчики расстраиваются из-за формы замечания, а не из-за содержания. Будьте вежливы, объясняйте аргументами, и скорее всего всё будет ок.
«Я исправлю это потом»
Если разработчик согласен с тем, что в коде есть проблема, но просит заапрувить MR, обещая, что исправит это в другой раз, то, по мнению ребят из Гугла, это «потом» чаще всего никогда не наступает. Поэтому если MR — не какой-то срочный багфикс прода, то нужно настаивать на исправлении.
Выводы
Мне очень понравился этот документ от Google. Особенно лайфхак со словом «nit», акцент на скорости code review, а также то, что MR не должен быть идеальным. Вроде бы просто, но пока это явно не обдумаешь, до дела не доходит.
Если эта статья покажется интересной читателям и наберет какое-то количество плюсов, то я напишу следующую часть: процесс code review со стороны автора MR.
Алсо, в ближайшем выпуске «Цинкового прода» мы подробно обсудим код ревью с разных сторон. Поэтому не забудьте подписаться на наш подкаст о разработке!
Еще одна статья о code review
Что такое code review
Что можно инспектировать
Для ревью подходит любой код. Однако, review обязательно должно проводиться для критических мест в приложении (например: механизмы аутентификации, авторизации, передачи и обработки важной информации — обработка денежных транзакций и пр.).
Также для review подходят и юнит тесты, так как юнит тесты — это тот же самый код, который подвержен ошибкам, его нужно инспектировать также тщательно как и весь остальной код, потому что, неправильный тест может стоить очень дорого.
Как проводить review
Вообще, ревью кода должен проводиться в совокупности с другими гибкими инженерными практиками: парное программирование, TDD, CI. В этом случае достигается максимальная эффективность ревью. Если используется гибкая методология разработки, то этап code review можно внести в Definition of Done фичи.
Из чего состоит review
Также очень важно определиться, за кем будет последнее слово в принятии финального решения в случае возникновения спора. Обычно, приоритет отдается тому кто будет реализовывать код (как в scrum при проведении planning poker), либо специальному человеку, который отвечает за этот код (как в google — code owner).
Как проводить design review
Design review можно проводить за столом, в кругу коллег, у маркерной доски, в корпоративной wiki. На design review тот, кто будет писать код, расскажет о выбранной стратегии (примерный алгоритм, требуемые инструменты, библиотеки) решения поставленной задачи. Вся прелесть этого этапа заключается в том, что ошибка проектирования будет стоить 1-2 часа времени (и будет устранена сразу на review).
Как проводить code review
Можно проводить code review разными способами — дистанционно, когда каждый разработчик сидит за своим рабочим местом, и совместно — сидя перед монитором одного из коллег, либо в специально выделенным для этого месте, например meeting room. В принципе существует много способов (можно даже распечатать исходный код и вносить изменения на бумаге).
Pre-commit review
Данный вид review проводится перед внесением изменений в VCS. Этот подход позволяет содержать в репозитории только проверенный код. В microsoft используется этот подход: всем участникам review рассылаются патчи с изменениями. После того как собран и обработан фидбэк, процесс повторяется до тех пор пока все ревьюверы не согласятся с изменениями.
Post-commit review
Данный вид review проводится после внесения изменений в VCS. При этом можно коммитить как в основную ветвь, так и во временную ветку (а в основную ветку вливать уже проверенные изменения).
Тематические review
Можно также проводить тематические code review — их можно использовать как переходный этап на пути к полноценному code review. Их можно проводить для критического участка кода, либо при поиске ошибок. Самое главное — это определить цель данного review, при этом цель должна быть обозримой и четкой:
Результаты review
Сложности при проведении review (субъективное мнение)
Основная сложность, с которой мы столкнулись, когда внедряли review в первый раз: это невозможность контроля изменений по результатам review. Отчасти это связано с тем, что данная практика применялась без других практик — CI (это еще раз доказывает тот факт, что все инженерные практики должны применяться вместе).
Утилиты для review
Вообще, утилит для проведения review существует большое количество, как платных, так и бесплатных. Я не стал их приводить, чтобы не навязывать свою точку зрения, в интернете можно найти множество инструментов и подробное описание.
Ссылки
Пожелания, дополнения, критика приветствуется
Код-ревью в Практикуме: как мы делаем его быстрее и эффективнее
Код-ревью — полезный инструмент для командной разработки и для прокачки собственных навыков. Код-ревью помогает обнаружить недочёты в коде: как синтаксические или стилистические ошибки, так и неоптимальные или неэффективные подходы. В командной разработке, когда команда делает большой проект, код-ревью также помогает оставаться в курсе изменений в разных частях кода.
Когда программист-новичок впервые сталкивается с код-ревью, он часто расстраивается. В его коде по неопытности часто оказывается довольно много проблем, которые видит более опытный разработчик. Однако очень важно уметь правильно принимать обратную связь, ведь задача ревьюера такая же, как и задача разработчика — сделать код проекта наиболее качественным и эффективным.
Меня зовут Артём Коломацкий, я старший ревьюер бэкенд-факультета в Яндекс.Практикуме. Я расскажу про практики, которые мы используем в код-ревью наших студентов. Часть из них — наши внутренние правила, а часть — универсальные советы, которые вы легко сможете применить у себя в команде.
Код-ревью в Практикуме
В Практикуме мы проводим ревью кода на собственной онлайн-платформе, которая называется «Ревизор». Туда попадают все сданные студентами работы. Платформа работает по аналогии с интерфейсами в Gitlab/Github/Bitbucket: можно просмотреть список файлов, изменения между версиями, а также оставить комментарии к определённым строкам.
Кроме того, код студента можно скачать, чтобы запустить и проверить на локальной машине, а также при желании оставить комментарии непосредственно внутри файлов, чтобы потом импортировать их обратно на платформу. Это сделано для тех, кому удобнее смотреть код в собственном редакторе или IDE.
Так выглядит страница, на которую попадает ревьюер, когда начинает проверять работу. Тут мы видим вкладку «Задание» — в ней приведён текст задания. Папки и файлы студента можно увидеть и прокомментировать на вкладке «Код-ревью», а во вкладке «История» посмотреть предыдущие варианты этой работы студента.
Во вкладке «Код-ревью» можно оставлять разные типы комментариев к коду.
Ну а в последней вкладке видно все предыдущие варианты с комментариями.
Интерфейс у студента очень похож на интерфейс ревьюера: видно комментарии к файлам и общий комментарий. Также есть возможность оставить отзыв на работу ревьюера. Всю обратную связь мы внимательно обрабатываем и разбираем вместе с ревьюерами.
Главное отличие «Ревизора» от других платформ в том, что у нас комментарии делятся на три типа: «Надо исправить», «Отлично» и «Можно лучше».
Что значат эти типы комментариев? «Надо исправить» — это обязательные правки. К ним относятся несоответствия заданию, грубые стилистические ошибки, неверное использование тех или иных инструментов и подходов.
«Можно лучше» — это о небольших помарках, а также о том, что не даётся в теории курса. Важный момент в том, что мы не требуем от студентов сразу писать идеальный код, мы двигаемся к этому постепенно с течением курса, поэтому наши ревьюеры знают, что покрыто теорией, а что нет, и могут дать дополнительную информацию или новые материалы для ознакомления в процессе ревью или при личном общении.
«Отлично» — это комментарии, которыми мы хвалим студентов за то, что они верно справились с заданием или использовали нестандартные и интересные подходы.
В Практикуме студенты учатся по спринтам. Спринт длится две недели, и за это время студент изучает законченную часть теории, например ООП в Python. По завершении спринта студенты пишут итоговый проект. Он небольшой, но позволяет студенту закрепить материал, а ревьюеру — убедиться в том, что студент усвоил теорию как следует.
Например, в случае спринта по ООП итоговый проект — это калькулятор валют. Так как в начале пути программиста очень важно привить культуру «красивого кода», мы особенно внимательно относимся к стилистическим ошибкам — соблюдению стайлгайда и повышению читаемости кода. Помимо этого мы следим за эффективностью и оптимальностью решения, например, что функция не потребляет лишней памяти или не делает запросов в базу, которых можно было бы избежать.
Как и в ревью на рабочем проекте, мы проверяем, что были выполнены поставленные требования, но если в случае продакшн-разработки это задача, поставленная менеджером, то у нас на курсе это задание итогового проекта спринта.
По нашим правилам один раз за отправку проекта на ревью студент может обращаться к ревьюеру с вопросами. Обычно таких отправок за проект получается от трёх до пяти штук. Так мы учим студентов правильно подходить к вопросам: прежде чем что-то спросить, нужно обработать проблему самостоятельно, разобраться в ней, подготовить вопрос и уже потом обратиться к ревьюеру. Вопросы к ревьюеру должны касаться только самих правок, например, если студент не понял, что именно требует ревьюер.
Если студент не понимает, как исправлять ошибку, он может обратиться к наставнику. Они занимаются помощью с решением финальных проектов и прохождением остальной теоретической части курса. Бывают ситуации, когда студент считает, что ревьюер не прав или что правка, которую он требует, — некритичная. Поэтому обязательное требование к ревьюеру — уметь грамотно объяснить необходимость того или иного исправления.
Формула идеального комментария
Постепенно мы пришли к внутренней формуле «идеального комментария».
Вот пример комментария:
А вот пример комментария «Отлично» за выбор эффективного решения:
Или пример необязательной правки:
Вот так выглядит хороший абстрактный комментарий, написанный по такой формуле:
Здесь лучше использовать кое-что другое, что реализует такой-то функционал, потому что вот это работает медленнее. Пример кода. Ссылка на бенчмарк, ссылка на пояснение «этого».
Эта формула позволяет вынести максимум пользы из ревью. Мы помогаем студенту найти правильную идею для реализации исправления, при этом он понимает, зачем нужна эта правка.
Но бывают случаи, когда студент и ревьюер всё равно не могут договориться. В продакшн-разработке тоже случаются такие случаи. Там принято обсудить проблему в комментариях к пулл-реквесту, затем обсудить её в личной переписке и, если не удается достичь консенсуса, привлечь старшего коллегу. Для ревьюеров «старший коллега» — это старший ревьюер, человек, который управляет всеми процессами, которые касаются ревью на факультете. В случаях недопонимания между ревьюером и студентом он включается в диалог и разрешает конфликт.
Принципы код-ревью
Первый и самый главный принцип: когда работа попадает к ревьюеру на проверку, он должен проверить её за сутки. Это важно, потому что процесс ревью блокирует дальнейшее обучение студента — без сдачи итогового проекта нельзя двигаться дальше по курсу.
Если работа кажется идеальной и хочется сразу её принять, нужно очень внимательно проверить её ещё раз — мы всегда стараемся предложить улучшения кода сверх программы курса и посоветовать что-то новое. Это помогает студентам получить больше полезной развивающей обратной связи.
Мы стараемся сохранять индивидуальность каждого ревьюера — не грести всех под одну гребёнку и давать простор для самореализации. Мы не хотим превращать ревьюеров в роботов, а хотим, чтобы студенты чувствовали, что их ревьюит живой человек, поэтому у нас нет шаблонных комментариев.
По схожей причине мы закрепляем студентов за одним ревьюером на весь срок обучения. Это нужно, чтобы студент и ревьюер привыкли к друг другу, открылись, ведь живой диалог позволяет генерировать более продуктивный фидбек. А ещё чтобы избавить студентов от стресса, ведь программирование — очень субъективная вещь, у каждого ревьюера свой бэкграунд, и некоторые подходы могут различаться.
Перед сдачей первого итогового проекта мы немного раскрываем студентам наши принципы код-ревью: учим их настраивать линтер (утилиту, которая проверяет код на стилистические ошибки) и рассказываем об общих требованиях к коду, на которые мы сами ориентируемся при проверке.
Мы стараемся переключать фокус внимания ревьюеров с незначительных ошибок на более крупные, для этого у нас есть так называемые чеклисты. Чеклисты — это текстовые описания возможных частых ошибок, которые допускают студенты в определённых проектах, с пояснениями, какие из них считать обязательными к исправлению, какие — нет, и где нужно дать студентам дополнительные материалы.
Кроме чеклистов у ревьюеров есть готовые решения проектов от старшего ревьюера — они написаны от лица среднего студента, который будет сдавать проект. В них нет лишнего неизвестного для студента функционала, а также присутствуют дополнительные комментарии для ревьюеров. Вдобавок к каждому проекту авторами написаны автотесты, которые проверяют основной функционал задания.
Недавно в пайплайн тестов, которые прогоняются на платформе при сдаче студентом работы, был добавлен линтер — теперь студентам недостаточно прогнать его локально. Пока все тесты на платформе не пройдут, работа не попадёт к ревьюеру.
За счёт всех этих факторов у ревьюеров появляется больше времени на более качественный фидбек студенту.
По фидбеку ревьюеров и студентов команда авторов регулярно обновляет контент курса: добавляются новые материалы, изменяются задания, а также дополняются тесты, проверяющие работы студентов.
Что почитать
Для тех, кто хочет повысить качество своего кода, я могу посоветовать книги Стива Макконнелла «Совершенный код» и Антона Спрола «Думай как программист».