Представим команду, где не проводится Code review. Разработчики пишут код и без проверок вносят все изменения в основную ветку. Спустя время расширяется функционал или находятся баги, они возвращаются к исходному коду, а там все переменные названы одной буквой, нет следования архитектуре, да и качество не самое лучшее. Этот некачественный код будет копиться и однажды наступит момент, когда при любом мало-мальском нововведении проект начнёт разваливаться: в лучшем случае, увеличится время разработки, в худшем – поддержка проекта станет невозможной. И это при том, что когда-то давно задача была выполнена и все хорошо работало.
Как этого можно избежать? Ответ на вопрос в названии – Code review.
Code review — это проверка кода на ошибки, повышающая стабильность работы и качество кода.
Теперь представим команду, где Code review уже проводится, но в процессе разработчики ругаются между собой, Pull request долго не одобряется, задачи начинают подвисать. Процесс от начала выполнения задачи до ее появления в проекте растягивается, а вместе с ним тормозится и вся работа.
Pull request / Merge request — это запрос к команде проекта (человеку или группе людей) на одобрение и применение изменений в выбранную ветку. Как только Pull request будет создан, перед одобрением происходит обсуждение написанного кода. Во время обсуждения могут быть предложены правки. После одобрения текущие изменения попадают в выбранную ветку.
Ниже перечислены рекомендации, которые помогут ускорить Code review и повысить его качество.
Поделим вопрос на три части и рассмотрим каждую по отдельности:
- Техническая часть: как проверять код и на что обращать внимание при проверке?
- Коммуникационная часть: как предупредить споры и снизить уровень стресса в ходе обсуждения?
- Организационная часть: что можно сделать, чтобы процесс Code review проходил эффективно?
Часть 1. Проверяем код
Запускаем код автора у себя
Запустите код у себя и посмотрите, как изменения работают в связке с остальным кодом. Это помогает найти проблемные места, которые не видны в web-интерфейсе. Старайтесь видеть код комплексно, избегайте фокусироваться только на локальных изменениях. Так Вы быстрее разберетесь с кодом и быстрее найдете архитектурные неточности, если такие есть.
Помним про пользовательский опыт
Обращайте внимание, как изменения в коде повлияют на конечного пользователя. Даже самый прекрасно написанный код может оказаться бесполезным для пользователей, что приведет к дополнительным задачам, багам, а также может нанести удар по репутации компании и продукта.
Смотрим на общую логику
Разработчики могут успешно решить свою задачу, но поломать работу других кусков кода. Чтобы такого не происходило, смотрите не только на то, как решается конкретная задача, но и на то, как изменения отразятся на работе других сервисов, модулей и всего проекта в целом.
Смотрим на архитектуру кода
Архитектура кода определяет, как много времени в будущем мы потратим на расширение, добавление функционала или правку бага. Также архитектура кода может повлиять на потенциальное появление багов в случае изменений в проекте. В идеале расширение и добавление нового функционала не должно приводить к рефакторингу.
Пишем проще
Обращайте внимание на переусложнение кода: чем проще код, тем легче он читается и проще его поддерживать. Избавляйтесь от сложных кусков кода.
Многопоточность
Если в проекте подразумевается работа в нескольких потоках, то смотрите, что будет если во время выполнения кода в каком-то из потоков случится задержка, и как отрабатываются подобные кейсы.
Излишняя оптимизация
Как писал классик Дональд Кнут, преждевременная оптимизация – корень всех зол. Оптимизировать лучше только то, что нужно здесь и сейчас.
Отрабатываем ошибки
Обратите внимание, как поведет себя проект, если не удалось выполнить строчку кода, блок кода или запрос на сервер. Часто разработчики прерывают выполнение функции без вывода ошибок, но подобные кейсы необходимо прорабатывать.
Соответствие договоренностям
Код должен соответствовать договоренностям, код стайлу. Единообразие кода – не прихоть, а необходимость: такой код легче поддерживать, и в таком коде легче разбираться.
Наименования и внешний вид
Помните о других программистах, которым придется разбираться в вашем коде. Читабельный код упрощает его дальнейшую поддержку. Имена должны быть понятные и точно описывать класс, объект, функцию и т.д.
Комментарии к коду
Комментарии должны отвечать на вопрос: «почему так сделано?», а не «как это сделано?». Запомните это.
Часть 2. Обсуждаем
Главное правило обсуждения: любой комментарий должен быть нацелен на код, а не на человека, который его написал. Работает и в обратную сторону – не стоит воспринимать комментарии как критику Вас лично. Задача code review сделать Ваш код лучше, ведь то, что не заметили Вы, могут заметить другие. Коллеги могут предложить альтернативные решения, и в этом заключен потенциал для профессионального роста. Важно помнить, что обсуждение кода – это не состязание в остроумии и не показательное шоу «кто больше знает», поэтому сарказм, скрытая агрессия и хамство в нем не уместны.
Как правило, pull request проводят на специальных web-хостингах (git.com, bitbucket.org, gitlab.com и др.), где есть возможность просмотреть изменения и оставить комментарий к определенному фрагменту кода.
Соблюдаем договоренности
Повторим, код должен соответствовать договоренностям. Однако если таких договоренностей не существует, не стоит просить коллегу добавить пробел или отступ в коде.
В спорных моментах можно договориться всей командой прямо в процессе code review, но просить соблюдать эти правила лучше на следующих code review, так всем будет легче их принять. Кстати, задокументированный гайдлайн по стилю написания убирает практически все споры.
Предлагаем альтернативу
Избегайте высказываний типа «ты сделал не так...», «зачем, почему ты так пишешь?» и др. Они воспринимаются как критика и ведут к оправданиям. Лучше писать комментарий про содержание кода, без обращения к личности автора. Также старайтесь избегать приказов и принуждений: люди не любят, когда им кто-то приказывает, и воспринимают такие комментарии негативно.
Можно действовать по следующей схеме:
- Пишем, что не так с кодом
- Почему так лучше не писать
- Предлагаем альтернативные варианты
Остаемся в рамках задачи
Часто можно увидеть комментарии к коду, который был раньше и никак не трогался. Не надо комментировать то, что не относится к задаче. Любые сторонние правки могут занять много времени, да и восприниматься могут негативно, поэтому лучше смотреть на то, как человек выполнил текущую задачу, а не просить его рефакторить проект.
Хвалим
Если видите интересное или крутое решение, не стесняйтесь хвалить. К тому же, это отличная мотивация для ваших коллег продолжать писать хороший код в дальнейшем.
Все комментарии равны
Часто бывает, что один программист технически знает больше другого, что подтверждается градацией junior, middle, senior, team lead. Не стоит выделять комментарии одной группы как более важные. Это обесценивает мнение части разработчиков, что может привести к равнодушию и формальному участию в code review. Когда мнение всех одинаково важное, code review проходит продуктивнее.
Четко выражаем свои мысли
Для продуктивного общения пишите максимально развернуто и объясняйте каждую деталь. У всех разный уровень знания, а читать мысли еще пока никто не научился.
Задаем вопросы
Не стесняйтесь спрашивать своих коллег, чем их предложенный вариант лучше текущего вашего. Это отличная возможность узнать что-то новое и вырасти профессионально.
Решаем конфликты
Случается, что человек не принимает все доводы и своих предложить не может, отказывается что-то делать. Несколько практических советов на этот случай:
- Большинство конфликтных ситуаций можно решить поговорив лично, в дружественном тоне.
- Вы можете уступить, так как Вы не можете писать код пока воюете.
- Можете привлечь всю команду и совместно решить, как лучше написать код.
- В текущем code review оставить все как есть, а спорные моменты вынести отдельными задачами по рефакторингу, именно задачами, потому что слова «потом исправлю», как правило, никогда не воплощаются в жизнь.
Часть 3. Улучшаем процесс
Описываем, что сделали
Описываем в заголовке pull request (или выносим в отдельный комментарий) суть задачи и какие шаги были проделаны для ее выполнения.
Делим pull request на части
Большой кусок будут долго смотреть, долго обсуждать и долго исправлять. Поделите код на небольшие логические части – так процесс пойдет гораздо быстрее.
Отвечаем на все комментарии
Желательно отвечать на каждый комментарий, чтобы в команде не возникало недоговоренностей. Другие разработчики должны понимать, что Вы прочитали их комментарий, проделали необходимую работу и внесли исправления. Постоянно открывать pull request и смотреть, что было поправлено, а что нет, очень неудобно и отнимает много времени.
Искать все?
Существуют разные подходы – искать максимум из возможного или комментировать сначала важные архитектурные моменты, а после исправления обращать внимание на мелочи.
Оба способа имеют право на жизнь. Я считаю, что второй вариант более трудозатратный: представьте, что после исправления надо полностью просматривать код еще раз, комментировать и снова ждать исправлений. Куда быстрее тщательно пройтись по коду один раз, оставить комментарии и потом проверить исправления.
Если есть архитектурные неточности и понятно, что мелкие ошибки сами исчезнут после исправления архитектуры, не стоит тратить время на комментирование мелочей в этом участке кода.
Ждать всех?
Можно не ждать пока pull request одобрят все и договориться, что одобрения 80% ревьюверов достаточно для закрытия задачи. Это ускорит попадание кода в основную ветку, что несомненно более выгодно для бизнес-процессов, но может привести к разногласиям в команде и равнодушию к code review.
Второй вариант – обязательно ждать одобрения всех причастных разработчиков. Качество кода вырастет, но сам процесс замедлится значительно. Выбор в пользу скорости или качества каждая команда должна принимать самостоятельно.
Мелочи
Если к коду нет серьезных замечаний, то не нужно ждать, когда будут убраны все маленькие неточности. Их можно указать в комментарии и сразу одобрить pull request – автор кода будет чувствовать себя спокойнее, повысит лояльность к команде, почувствует, что ему доверяют. И, конечно, скорость прохождения pull request возрастет.
Заключение
Code review это важная часть процесса разработки, которая сказывается на проекте в целом, поэтому оставлять ее без внимания опасно. Какие-то из этих рекомендаций помогут ускорить code review, какие-то – сделать его качественней. Надеюсь, мой опыт и знания окажутся полезными для читателей этой статьи.
Автор: VAnsimov
