что такое код ревью

Что такое код-ревью

Это проверка кода на ошибки, неточности и общий стиль программирования.

Послушать аудиоверсию этой статьи (6 минут):

Слушайте Что такое код-ревью на Яндекс.Музыке

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

Когда вы пишете новую функцию, она не попадает сразу в проект. Вместо этого ваш код отправляется на код-ревью (code review).

Что делают на код-ревью

Во время код-ревью кто-то из старших товарищей изучает ваш код на предмет:

Если проблемы есть, проверяющий отправляет код на доработку. Если всё хорошо, код переходит на следующую стадию — как правило, в тестирование.

Кто проводит

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

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

Как это выглядит

Зачем это нужно

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

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

Источник

Еще одна статья о 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 существует большое количество, как платных, так и бесплатных. Я не стал их приводить, чтобы не навязывать свою точку зрения, в интернете можно найти множество инструментов и подробное описание.

Ссылки

Пожелания, дополнения, критика приветствуется

Источник

Код ревью, как внедрить и не испытывать боль

Если вы работаете в продуктовой компании, то жизненный цикл почти каждого продукта будет соответствовать принципу Парето:

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 – зачем и как использовать в команде?

Что такое Code Review

Зачем нужен Code Review

Code Review может являться частью процесса выполнения задачи (частью workflow). Может показаться, что ревьювить должен только тимлид или старший разработчик, но хорошей практикой является если в процессе ревью задач участвуют все разработчики. Таким образом можно не только распределить нагрузку от ревью, но и составить у команды более широкое представление о выполняемых задачах. Также это помогает делиться best practices внутри команды.

Положительные эффекты в команде от Code Review:

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

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

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

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

при разработке задачи на реализацию тратится чуть больше времени

в задаче задействованы как минимум два разработчика (тот, кто делал задачу и тот, кто ее ревьювил)

Рекомендации по организации Code Review

Code Review может быть организован по-разному в разных командах. Главное, чтобы команда заранее обговорила и утвердила свои внутренние правила, которых она хочет придерживаться и с которыми все согласны, чтобы каждый раз не возвращаться к этому вопросу.

Избегать рутинных проверок Code Style людьми: автоматизировать такие проверки. Можно использовать для этого любые подходящие вам инструменты для автоматической проверки code style используемого вами языка программирования. Например, для PHP это может быть PHP Coding Standards Fixer https://cs.symfony.com/ Не нужно тратить время разработчиков на то, что можно автоматизировать. Также обратная связь по поводу code style от людей воспринимается как “придирки” и может создать не очень позитивную атмосферу в команде.

Code Review должен проводиться для каждого участника команды, вне зависимости от уровня. Не должно быть такого, что ревьювят только задачи, которые сделали Junior разработчики, тем временем Senior разработчики не отдают свои задачи на ревью. Необходимо, чтобы ревью проводилось для задач всех разработчиков.

Code Review является частью процесса и необходим каждой задаче. Это правило избавляет от лишних споров и холиваров насчет небольших задач. Ревью проходят все задачи без исключений.

Code Review проводится перед релизом задачи и перед передачей ее в тестирование. Это помогает избегать повторного тестирования, а также соблазна оставить все как есть, ведь “и так работает”. К задачам, которые уже на бою, никто не захочет повторно возвращаться.

Избегать слишком больших объемов кода в одном Code Review. Если задача большая, то необходимо отправлять ее на ревью частями. Есть рекомендуемое число 200-400 строк в одном ревью. При увеличении количества строк, эффективность и продуктивность ревью резко падает.

Если нет возможности внести какие-то правки после ревью, то необходимо завести задачу в трекере задач, а в коде оставить ToDo с ее номером

Чего следует избегать:

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

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

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

На что обращать внимание во время Code Review

Чеклист для разработчика перед отправкой на ревью:

Проверить, что реализация соответствует всем указанным в исходной задаче условиям

Проверить соответствие Code Style и другим принятым в команде гайдлайнам, например, наличию unit-тестов и документации

Протестировать задачу локально и убедиться, что она работает, как нужно

Подготовить описание для ревьювера, если какой-то информации в задаче не хватает

Проверить, нужны ли какие-то комментарии в самом коде и добавить при необходимости

Чеклист для ревьювера:

Ознакомиться и понять цель и суть задачи

Проверить общую архитектуру и подход к решению

Проверить мелкие детали (имена функций и переменных и т.д.)

Проверить наличие тестов и документации по необходимости

Список ToDo: изменения, которые необходимо внести в код после ревью

Вопросы: обозначить свои вопросы по частям кода, которые непонятны после ревью

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

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

Инструменты для Code Review

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

Источник

Code review по-человечески (часть 1)

В последнее время я читал статьи о лучших практиках code review и заметил, что эти статьи фокусируются на поиске багов, практически игнорируя другие компоненты ревью. Конструктивное и профессиональное обсуждение обнаруженных проблем? Неважно! Просто найди все баги, а дальше само сложится.

Так что у меня случилось откровение: если это работает для кода, то почему не будет работать в романтичных отношениях? Итак, встречайте новую электронную книгу, которая поможет программистам в отношениях со своими возлюбленными (обложка на иллюстрации слева).

Моя революционная книга обучит вас проверенным техникам по выявлению максимального количества недостатков в своём партнёре. Книга не затрагивает следующие области:

• Обсуждение проблем с сочувствием и пониманием.
• Помощь партнёру в устранении недостатков.

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

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

Так почему мы проводим code review таким образом?

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

Я собираюсь сделать смелое предположение, что вам хотелось бы улучшить code review в настоящем, где вы работаете с людьми, а не с роботами. Даже более смелое предположение, что хорошие отношения с коллегами — это главная цель, а не просто переменная для ускорения исправления ошибок (минимизации параметра cost-per-defect). Как бы изменились ваши методы ревью с учётом этих обстоятельств?

В этой статье обсудим техники, которые предполагают, что code review — не только технический, но и социальный процесс.

Что такое code review?

Термин “code review” может означать разные действия, от простого чтения какого-то кода из-за спины разработчика до совещания на 20 человек, где вы разбираете код строчка за строчкой. Я здесь имею в виду формальную и письменную процедуру, но не отягощённую рядом совещаний для инспекции кода.

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

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

Ревью начинается, когда автор отправляет список изменений рецензенту. Оно происходит в несколько раундов. Каждый раунд — это один полный цикл приёма-передачи между автором и рецензентом: автор присылает изменения, рецензент отвечает отзывом на эти изменения. Каждая рецензия кода состоит из одного или нескольких раундов.

Ревью завершается, когда рецензент одобряет изменения. Обычно этому сопутствует отправка сообщения LGTM, сокращённой фразы “looks good to me”.

Почему это так трудно?

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

«Это одна из причин, почему я не скучаю по IT, ведь программисты — весьма малопривлекательные люди… Например, в авиации те, кто слишком переоценивают свои способности, уже мертвы».

Филип Гринспан, сооснователь ArsDigita, из книги «Основатели за работой».

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

Как будто и без этого недостаточно сложностей, здесь вами приходится выражать свои мысли в письменной форме, что ещё больше повышает риск недопонимания. Автор не слышит тон вашего голоса, не может воспринять язык тела, поэтому так важно аккуратно подбирать формулировки. Для автора, который занял оборонительную позицию, безобидное примечание вроде «Ты забыл закрыть файловый дескриптор» может звучать как «Не могу поверить, что ты забыл здесь закрыть файловый дискриптор! Ты такой идиот!»

Техники

Отдайте компьютерам скучную часть работы

Когда вас постоянно отвлекают совещания и почтовые письма, трудно выкроить время для вдумчивого анализа кода. Ментальных сил ещё меньше, чем времени. Чтение кода коллеги — когнитивно тяжёлая задача и требует высокого уровня концентрации. Не транжирьте эти ресурсы на задачи, которые может выполнить компьютер, особенно если он выполняет их лучше.

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

Правая часть пустая, потому что автор использовал редактор кода, который автоматически форматирует пробелы каждый раз при нажатии кнопки «Сохранить». Ну худой конец, когда автор отправляет свой код на проверку, система непрерывной интеграции сообщает о неправильных пробелах. Автор исправляет проблему ещё до того, как рецензент её заметил.

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

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

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

Нужно всем вместе поработать, чтобы внедрить автоматические проверки такого рода в рабочий процесс code review (например, хуки перед коммитами в Git или вебхуки в GitHub). Если процесс ревью требует от автора запускать такие проверки вручную, то вы лишаетесь большей части преимуществ. Автор неизбежно иногда будет забывать о проверке, из-за чего вам придётся возиться с с простыми ошибками, которые могла исправить автоматическая проверка.

Оформите аргументы по стилю в виде руководства по стилю

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

Хорошее руководство по стилю определяет не только внешние элементы оформления вроде соглашения о присвоении имён или правила отступов, но и как использовать функции данного языка программирования. Например, JavaScript и Perl набиты функциональностью — там есть много вариантов реализации одной и той же логики. Руководство по стилю определяет Один Правильный Способ программирования, так что вы больше не увидите, что одна половина вашей команды использует один набор функций языка, а другая половина — совершенно другой набор функций.

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

Вариант 1: адаптация существующего руководства по стилю
Если поискать в интернете, то можно найти опубликованные руководства по стилю — и позаимствовать их для собственного использования. Самые известные — руководства по стилю от Google, но можно найти и другие, если эти не подходят. Адаптируя существующий документ, вы получаете все выгоды от наличия руководства по стилю, не тратя времени на написание такого документа с нуля.

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

Вариант 2: Постепенно дополняйте собственное руководство по стилям
Если не хотите адаптировать существующий документ, можно создать свой собственный. Каждый раз, когда во время код ревью возникает дискуссия по стилю, поднимайте перед всей командой вопрос, каким должно быть официальное соглашение. Когда достигнете согласия, закрепляйте это решение в руководстве по стилям.

Я предпочитаю держать наше руководство в формате Markdown в системе контроля версия (например, на GitHub Pages). Так все изменения проходят через стандартную процедуру рецензирования — кто-то должен явно одобрить изменения, а каждый в команде может поднять любую проблему. Вики и Google Docs тоже подходят.

Вариант 3: Гибридный подход
Сочетая варианты 1 и 2, вы можете адаптировать существующее руководство и одновременно вести локальный документ для расширения или перезаписи базы. Хороший пример — руководство Chromium C++. Там в качестве базы взяли руководство по C++ от Google, но сделали собственные изменения и дополнения.

Начинайте ревью немедленно

Расценивайте code reviews как задачу с высоким приоритетом. Когда вы изучаете код или пишете отзыв, не торопитесь, но начинайте делать это немедленно — в идеале, в течение нескольких минут.

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

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

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

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

Начните с высокого уровня, и спускайтесь ниже

Чем больше заметок вы пишете в каждом конкретном раунде, тем больше риск, что автор будет чувствовать себя подавленным. Точный лимит зависит от разработчика, но опасная зона обычно начинается в районе 20-50 заметок на один раунд ревью.

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

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

Щедро используйте примеры кода

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

Хороший способ улучшить восприятие код ревью автором — найти возможность подарить ему подарок. А какие подарки любят получать все разработчики? Конечно, примеры кода.

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

Предположим, ваш автор не очень хорошо знаком с функцией списковых включений в Python. Он присылает на рецензию код с такими строчками:

Ответ в стиле «Можно это упростить с помощью списковых включений?» вызовет раздражение, потому что ему нужно потратить 20 минут, изучая функцию, которую никогда раньше не использовал.

Автор будет гораздо счастливее получить такую заметку:

Предлагаю рассмотреть списковое включение такого рода:

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

Применяйте эту технику только для ясных, бесспорных улучшений. В вышеприведённом примере спискового включения немногие разработчики будут спорить с сокращением количества строк кода на 83%. И наоборот, если вы напишете пространный пример для демонстрации изменения, которое «лучше» на ваш личный вкус (например, изменения стиля), то такой пример кода создаст впечатление, что вы настырно проталкиваете свои предпочтения, а не проявляете щедрость.

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

Никогда не говорите «ты»

Это звучит немного странно, но послушайте: никогда не обращайтесь лично к автору в процессе код ревью.

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

Подбирайте слова для отзыва таким образом, чтобы минимизировать риск возникновения оборонительной позы. Чётко обозначьте, что вы критикуете код, а не самого разработчика. Когда автор видит в комментарии личное обращение к нему, это отвлекает его внимание от кода и переводит внимание на личность. Так повышается риск, что он воспримет критику лично.

Например, вот безобидный комментарий:

Ты допустил ошибку в слове «благополучно».

Автор может интерпретировать такое замечание двумя соврешенно разными способами:

Вторая заметка — это просто исправление, а не оценка автора.

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

Вариант 1: Замените «ты» на «мы»

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

Вариант 2: Удалите из предложения субъект
Другой вариант избежать личного обращения — использовать сокращение, которое исключает из предложения субъект:

Того же эффекта можно достичь с помощью пассивного залога. Я обычно в технических текстах избегаю пассивного залога как чумы, но это может быть полезный вариант обхода проблемы с личным обращением:

Ещё один вариант — перефразировать предложение в виде вопроса, который начинается со слов «Что насчёт. » или «Как насчёт. »:

Оформляйте отзывы как запросы, а не команды

Код ревью требует большего такта и осторожности, чем обычное общение, потому что здесь выше риск, что обсуждение скатится в личный спор. Казалось бы, рецензенты должны проявлять бóльшую вежливость и учтивость в код ревью по сравнению с личным общением. Но я обнаружил страннейшим образом прямо противоположную ситуацию. Многие люди никогда не скажут коллеге, «Дай мне этот степлер, а потом принеси газировки». Но я видел множество случаев, когда рецензенты оформляют отзывы в таком командном стиле, вроде «Перенеси этот класс в отдельный файл».

Но и не будьте раздражающе вежливым в своих комментариях. Оформляйте их как запросы или предложения, а не как команды.

Сравните одно и то же замечание, оформленное двумя разными способами:

Людям нравится контролировать свою работу. Такой запрос даёт автору чувство самостоятельности.

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

Сравните, насколько воинственной кажется беседа в зависимости от оформления первоначального замечания:

Видите, насколько более цивилизованным стало общение, когда вы сконструировали воображаемый диалог в доказательство своего тезиса оформили замечание в виде запроса, а не команды?

Обосновывайте принципами, а не мнениями

Когда составляете замечание для автора, то объясните и предлагаемое изменение, и его причину. Вместо фразы «Нам следует разделить этот класс на два» лучше сказать «Сейчас этот класс отвечает одновременно и за скачивание файла, и за его парсинг. Следует разделить его на класс для скачивания и на класс для парсинга согласно принципу единой ответственности».

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

Разработка программного обеспечения — это одновременно искусство и наука. Не всегда можно точно сформулировать, что именно неправильно в данном фрагменте кода с точки зрения устоявшихся принципов. Иногда код просто некрасивый или излишне усложнённый, и сложно точно сформулировать, почему. В таких случаях объясните как можете, но сохраняйте объективность. Если сказать «Мне кажется, это сложно понять», то это хотя бы объективное утверждение, в сравнении с фразой «Это запутанный код», что является оценочным суждением, с которым не каждый может согласиться.

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

Часть 2: скоро

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

Источник