Прикладное руководство по внедрению code review

1. Что такое код ревью
2. Психология проверяющего и проверяемого
3. С чего начинается код ревью?
4. Регламенты и временные рамки
5. Процесс ревью
6. Ссылки на примеры

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

Уже написано и перенаписано множество статей на эту тему. Просто давайте быстренько закрепим профит от этого и пойдём дальше.

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

Теперь так же, без лирики, к подводным камням

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

Надеюсь вышесказанное убедит вас хотя бы попробовать внедрить эту практику на своём проекте.

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

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

Психология проверяющего и проверяемого

Нельзя просто так взять и написать коммент.

И так, у нас уже есть code-style команды, cheklist ревью и книга золотых практик работы с нашим языком слэш фреймворком. Так в чем проблема? А проблема в том, что сейчас мы будем просматривать ПР выстраданный часами (а иногда и днями) беспрерывной работы, такой красивый и хороший, терпеливо росший в заботливых руках. И тут мы со своим списком правок...

Речь пойдёт о самых мягчайших из soft skills - умении подобрать слова для описания комментариев. Да, в идеальном мире автор реквеста принимает указания исключительно как зону роста и развития. Но это только сферический разработчик в вакууме. И хотелось бы сказать, что чувствительно сердце только в ранний джуновский период, а зрелые мидлы застрахованы от обиды. Но нет, критику можно плохо воспринимать в любом возрасте. Ревьюер не может повлиять на толщину кожи ревьюируемого. Так что же зависит от нас?

Приведу пару примеров

• Исправь это.

• Это лучше написать иначе.

• Ты не знаешь, как это писать?

• Мы делали похожий код, можешь посмотреть его тут.

Намек понят? Теперь правила в сухом остатке

1. Уважайте друг друга. ПР - не место для искрометного юмора или сарказма
2. Прицеливайся. Размытое "это надо исправить" не поможет решить проблему. Четко указывай область исправления.
3. Старайся пояснять почему код нужно исправить - это поможет вырасти вам обоим.
4. Критикуя предлагай. Указывая направление, в какую сторону должен меняться участок кода ты сильно ускоришь процесс. Тут есть два подхода: если ты только подталкиваешь к решению, то это поможет разработчику вырасти, если чётко формулируешь решение - сокращаешь время к исправлению до минимума.
5. Все холеварные вопросы (не блокирующие реквест) помечай префиксом NIT (сокращение от nitpick).
6. От себя, на практике - если ревьюер спрашивает автора, "что он хотел сказать этой строчкой", то автору лучше переписать этот код так, чтобы вопрос отпал.

И ещё, есть прекрасный принцип баланса - чтобы у автора МР не было тягостного ощущения собственной слабости, хвалите его за удачные решения. 

С чего начинается код ревью

- Вы готовы дети?
- Да, капитан.  

Команда должна быть готовой к практике ревью - все должны чётко понимать зачем, как и когда:

1. вся команда должна понимать, что ревью это не прихоть, а полезная необходимость;
2. у команды должен быть базис из code convention и чек-листа, в идеале созданные совместными усилиями;
   
3. должна быть договорённость о том что выложенный реквест идёт с пометкой ASAP - нельзя откладывать ревью на неопределённый срок.

Code convention - внутренняя договорённость команды о правилах написания кода, стандарт его оформления. Обычно она включает в себя метод выбора имени и регистра, стандартизация спорных мест написания кода.

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

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

Регламенты и временные рамки

Для того чтобы поставить такие жёсткие рамки для процессов мерджа на проекте вводились следующие правила

1. Оповещаем об открытии ПР. Как рекомендация - создать отдельный чат-ревью (особенно если основной чат наполнен прочей информацией) и как только открытый реквест готов к проверке - отправлять ссылку.
2. Теперь команда знает, что у них есть условные 3 часа, за которые ты должен найти время на просмотр ПР. Открывший реквест разработчик в это время смотрит другие реквесты и подготавливается к решению следующей задачи. Но в идеале не начинать новую таску до окончания текущей.
3. После проведения ревью нужно оповестить автора о том, что есть изменения. Часто это решается реакцией на оповещение (сообщение с ссылкой).

А дальше новая итерация ревью исправлений, начиная с пункта один.

Процесс ревью

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

1. правильно ли спроектирован код;
2. в правильном ли месте он расположен;
3. код функционирует и решает задачу;
4. соответствует ли принятым правилам стиля и нейминга.

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

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

Материалы и ссылки

Оставлю несколько ссылок - пример действующих сейчас на проекте code convention и чек-лист ревью с уклоном в Angular.

Надеюсь эта статья будет полезной как на этапе внедрения этой чудесной практики, так и на этапе развития и масштабирования!