Agile Base Camp 29 Мая в Киве: что я там увидел (Часть 2. Code Review)

Второй темой, которую я очень люблю, была практика ‘Code Review’. Доклад назывался “Применение практики ‘Code Review’ для улучшения качества продукта” и вели его Николай Алименков и Алексей Солнцев. Ребята поделились своим опытом применения этой практики и множеством вкусностей, часть из которых я успел записать. Напомню, что это всего лишь моя скромная репрезентация доклада. Так же отправляю заинтересованного читателя на первую странчику этого обзора. Читаем дальше...

Code Review проводить обязательно надо. Команда может попробовать практику и сказать "ну мы попробовали раз - нам не понравилось". Конечно не понравится! Столько, ведь, тонкостей в этом инструменте. Вообще ребята советовали, раз уж на ретроспективе приняли решение ревьювить код, то задача менеджера всю итерацию дергать народ за код ревью. Приняли демократичное решение, а пробивать в жизнь его надо авторитарно.

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

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

С усталостью и вниманием связанны некоторые исследования, проведенные какой-то конторой для какого-то кода. На цифры ребята не очень советовали полагаться, а уделить внимание тенденции: есть некоторое количество строчек кода, просматривамое ревьювером за час, когда ревьювер еще полезен; есть так же и оптимальная продолжительность ревью; а так же оптимальное количество строк на одну ревизию (коммит). Эти все цифры получаются опытным путем.

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

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

Раз уж зашел разговор за парное программирование, то отмечу один термин, который мне понравился. "Парное программирование для бедных" - это когда ты мне код ревьювиш, а я тебе. Возможно так же и другие алгоритмы выбора ревьювера: к примеру можно создать общий чат, в который тот, кто готов к код ревью пишет "на ревьюшечку!" и свободный в данный момент времени становится ревьювером. Можно проводить код ревью между командами - очень хороший способ избежать замыливания глаза одной команды, как результат весьма вероятно получить фидбек типа: "вы еще до сих пор используете QWE, ну вы даете - мы уже давно вот юзаем ASD". Так же можно создать S.W.A.T. команду, которая будет разбивать в пух и прах любой код любой команды (хочу в такую).

Ребята призывали делать ревью кода до его отправки в реппозиторий. В противном случае это будет не код ревью, а тестинг - для баги, найденной в реппозитории есть бактрекинг система, и все тут.

Если после первого неудачного ревью исправленный код снвоа зафейлил ревью - этот код стоит считать опасным и выделить на его доработку/тестирование дополнительное время.

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

В другом подходе, когда ревью ведет ревьювер, есть свой плюс - код проглядывает человек который никогда его не видел. Эту стратегию классно описывает карикатура от гугла:

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

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

Есть множество спец тулзов для улучшения code review, как платных так и бесплатных. Их стоило бы испробовать...

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

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

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

Вот так вот.

На сегодня все. Продолжение следует...