×Закрыть

Code review

Понимаю что такая тема может быть оффтопом на ДОУ, но все-таки отвлеку сообщество от обсуждения ЗП и судьбы очередного джуна.

Интересует как у вас в организации/команде проводится данный процесс: какие тулы используете, насколько «обязательные» ревью, сколько времени занимает и тд.
Какие плюсы/минусы получили? Какие сложности были?
Если таки не внедрили, то почему?

Интересует личный опыт, теоретические статьи я читал :)

LinkedIn
Допустимые теги: blockquote, a, pre, code, ul, ol, li, b, i, del.
Ctrl + Enter
Допустимые теги: blockquote, a, pre, code, ul, ol, li, b, i, del.
Ctrl + Enter

github pull request? И комменты.

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

Я один тут, как нормальный человек из Idea делаю код ревью?

Я один тут, как нормальный человек из Idea делаю код ревью?
А замечания куда пишете?

Кричу матом на всю комнату.

p.s. А если серьезно, то записываю в текстовый файл.

p.s. А если серьезно, то записываю в текстовый файл.
А потом файл выкладываете на тот же ФТП (или на тот же флопи) кде лежит код :)

У меня был таск на Nokia online проекте — делать code review для команды индусских программеров. ржал много :)

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

что цель и результат успешного сабжа — готовность ревьюера поставить подпись под каждой строкой кода.

Ну, цель таки — качественный код, а то что вы описали — это, скорее, сайд эффект.

По моему скромному мнению лучше написать стандарт кодирования (выделить чьи-то человеко-часы для этого). И после обсуждения и принятия командой — пусть каждый сам пишет код по описанным стандартам.

Коде ревью — это попытка на «народном виче» принять решение по каждому пустяку, вместо готового закона.

Коде ревью — это попытка на «народном виче» принять решение по каждому пустяку, вместо готового закона.

Неправильно. Садись, два.

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

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

какие тулы используете

Текстовый редактор для написания отчета.

насколько «обязательные» ревью

Незнаю — никогда не отказывался от такого полезного процесса.

сколько времени занимает

Часа два-три.

Какие плюсы/минусы получили?

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

phabricator.org никто не пробовал в живых проектах?

использую git как review tool для бедных, что-то вроде так:
git merge —squash #смержить все коммиты на ревью для удобства просмотра в целом
git commit * n # комменты или подправки — как новые коммиты поверх

git rebase —onto <original_review_revision> && git push # перетянуть эти новые комменты и коммиты назад и запушить

плюс — не нужно никаких специальных тулзов

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

пробовали разное. в данном случае описывался простейший способ с

плюс — не нужно никаких специальных тулзов
С gerrit-ом сам не работал, но соседние комманды жаловались на очень негибкий workflow.

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

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

Когда в команде у каждого свой стиль кода — это плохо. А когда есть Code Conventions — тут и критики особой нет.

Когда в команде у каждого свой стиль кода — это плохо
Главное — это чтобы код читался (скажем, отсутствие 9-ти этажных методов и перенос длинных строк) Например, если кто не может понять как оно работает из-за отсутствия (или присутствия) скажем, abstract и base в имени абстрактного класса, или кому глаза режет венгерская нотация — перечитать книгу по языку еще раз (или сходить к окулисту :)

если у вас основные претензии — только к разношерстным стилям, то это круто.

У кого-нибудь есть опыт работы с Google Code Reviews/Rietveld? Особенно интересно узнать, пробовал ли кто-нибудь развернуть его на локальной машине.

Пробовали. Впечатление — «костыль».

Имеем поток относительно небольших проектов, код в GitHub.
Идет ревью всех коммитов лидом по GitHub News Feed, очивидные комментарии там же, более важные/спорные — на месте за компом. Стараюсь уговорить каждого ревьюить каждого, пока выходит плохо(

Сборка проектов — Jenkins, в планах туда же добавить Solar + Gerrit для код ревью.

Есть ли у кого опыт использования Gerrit??

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

Пример: Google Android — проект опенсорсный, но каждый патч прежде чем попасть в репозиторий проходит через Gerrit — инструмент для code review, и мержится лишь по полючению определённого количества плюсов.

Ещё как получится и будет более эффективным средством, чем наличие «Процесса».

Сомневаюсь. Сначала забили на ревью потому что спешили, потом потому что релиз, потом потому что Вася высмеял тех кто парится, а потом потому что этого никто не делает ну и я не буду. И всегда будут те кто будет называть это «пиндосскими выдумками» и «детскими игрушками»...

Если пункт «уговорить» выполнен, такого не случится, т.к. все будут эту активность, если не любить, то принимать и понимать. То, что вы описываете, это когда этот пункт не выполнен.

По поводу вашего «сомневаюсь»:

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

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

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

Мы пользовались nvie.com/...ranching-model, там как раз основная разработка велась в одном бранче ’dev’, коммиты в который, собственно ревьюились.

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

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

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

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

В виду описанного выше, ревью происходит разными способами:
+ Подойти и попросить «пошли код покажу =)»
+ Лично я просматриваю изменения всей команды
+ Раз в неделю у нас есть митинг, куда мы «приносим», что-то на ревью, длина около часа.
+ Забавные, непонятные участки отправляем по почте, сохраняем дискуссию потом.

+ Иногда создаём презентации (да да PowerPoint) для демонстрации чего-то эдакого (удобно стрелочки рисовать)

Команда из разных людей — сильных и взрослых (40+), молодых и способных (23+). За всё время экспериментов пришёл к выводу, что гораздо более важным является добиться понимания, что ревью это не просто нужно, а ещё и полезно и весело. Когда люди начинают это чувствовать, процесс происходит сам собой, то там, то тут, то одним, то другим способом. Становится обычной каждодневной активностью.

Тем не менее идеальным иснтрументом я вижу следующее:
+ плагин для IDE, который имеет интерфейс как у Review Tools from MS Word
+ результаты хранятся в сорс контроле

+ доступен веб интерфейс с подсветкой кода и навигацией близкой к привычной в IDE

Даже было начал писать подобное решение, но пока подзабил.

> какие тулы используете

crucible

> насколько «обязательные» ревью

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

> сколько времени занимает

установка на чтобы занимало <=0.5 часа в день. В подавляющем большинстве случаев этого хватает.

> какие плюсы/минусы получили?

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

Как впечатления от crusible`а?

В целом очень положительные!

Хотя мелочи конечно есть всякие. Как по мне не хватает наглядности при работе с комментариями. Например комментарий можно пометить, что «вот тут баг», указать приоритет и тип. Но как потом посмотреть список моих комментариев с пометкой «дефект», посмотреть ответил ли на них автор ревью (у нас принято потом отписаться в стиле «пофикшено»). Или просто посмотреть все комментарии для всех ревью которые еще не summarize-ны, там иногда развивается дискуссия а через почтовые уведомления следить за ней неудобно... В общем лично мне чуть не хватает обратной связи.

мы в jira ведем обратную связь, ревью в крусибле — вторично и довесеок к тикету

Поддержу Нестора. Модератор просматривает все комменты и выносит решение. Решение пишется в ревью и может дублироваться в джиру.

Хорошо, правда не всегда удается нормально изменения запихнуть туда, но это терпимо. Удобный инструмент.

правда не всегда удается нормально изменения запихнуть туда

А какие проблемы могут возникнуть? Какого рода?

У нас были проблемы с патч-файлами в случае сложного дерева веток и меток. Решилось просто — стали делать патчи в эклипсе на уровне проекта.

Одну уже сказали, вторая — невозможность нормально скопипастить код при ревью.

Объясните, пожалуйста, подробнее. Не могу понять проблему.

crucible

это просто «Das ist fantastisch!» по сравнению с тем что я ещё повидал. Опять же инеграция в джиру и прочие ихние проекты. must have.

review после коммита, иногда параллельно с тестированием
тулзов никаких, простой просмотр изменений в свн
времени примерно 5-10% от разработки
помогло избавиться от большинства багов, сделанных по недосмотру, либо по незнанию

теоретически обязательно :) , реально по разным причинам может быть отложено (очень редко)

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

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

— кому-то было скучно, и он решил провести ревью — такое тоже случается

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

Ревью может провести любой участник проекта, но обычно ревьюит кто-то с опытом.

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

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

Используем переделанный под свои нужнды CollabNet TeamForge.

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

Для нового кода обычно используются один, максимум два человека, которые могут сделать code review, но обычно до релиза никто код не смотрит.

Времени занимает немного, т.к. много людей, главное не тянуть одеяло на себя и не учавствовать во всех ревью :)

Сложностей не знаю, пришёл на готовое :) Минусов я не нашёл. Из несомненных плюсов, ощущение полного контроля над кодом, очень тяжело кому-то «запороть» или внести критические изменения в ответственные участки кода.

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

Минусов нет.

Раскажу о «нерабочем» проекте — cuptrek.com
Команда состоит из 4 разработчиков и одного менеджера/тестировщика.

Разработка проекта ведется вечерами (каждый у себя дома) и на выходных (собираемся вместе, пеир программинг и обсуждение проблем/решений за чашечкой чая/кофе)

При разработке придерживаемся следующих правил:
1. Новый функционал/баг проходит следующие етапы жизни:
— разработка/фикс на специально созданом бранче
— ревью измений на бранче с анатацией замечаний в коментариях задачи
— «гарячее» обсуждение замечаний
— исправление замечаний
— мерж на транк
— тестирование на стейджинге

— выкат на лайв.

2. Тесты пишем только на ЛОГИКУ и на баг для фикса. Тоесть перед фиксом баги пишется тест, который эту багу выявляет и в последствии этот тест должен пройти.

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

После 2 недель обкатки такого, нового для нас, процеса мы признали его идеальным для нас.

Тоесть по сути вместо дополнительных тулзовин добавили статус «код ревью» к задаче. Соответвенно ревьювер просматривает и пронюхивает код. Оставьляя свои впечатления в коментариях к задаче.

Личный опыт. Текущее место работы
1. Код ревью обязательны ( комиты без ревью — исключение, происходит такое когда кто нибуть фиксит билд ночью или при подобной жести когда просто некого позвать а комитить надо сейчас. Бывает редко. )
2. Занимает от задачи. От 5 минут до рабочего дня. Время на ревьювера считается в тикет
3. Когда ревью не делается на одном компьютере или для ревью уже после комита исползуем crucible
4. Плюсы- откровенные косяки не комитятся. Они бывают у всех.
5. Сложности — некторых задротов не любят звать на ревью. Кое кто имеет мнение но не компетенцию их тоже не любят звать. В итоге из тима в 10 человек ревью регулярно делает половина.

По моим ощущениям- слабенькая замена настоящему XP с парным програмированием. На безрыбье покатит.

для ревью уже после комита исползуем crucible

Почему не использовать ревью и для pre-commit? Похоже, в вашем случае оно бы помогло решить и проблему

Сложности — некторых задротов не любят звать на ревью.

Да, и нельзя позиционировать CR, как замену XP в общем и парному программированию в частности. Это просто разные инструменты.

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

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

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

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

Помогает следить за одним кодстайлом в проекте.

В будущем планируем Jenkins + Phing — тесты, веб морда, код стайл.

Почти без ревью, тулов никаких, организации нет, будет ревью если сам попросишь, естественно таки образом совершенно не ревьюится г о в н о(спасибо за фильтр «ужасных» слов)код. Если что-то и делается то от этого не много толку, всё равно куча людей даже не проверяет компилируются ли их код и заливает как есть. Так жить нельзя, а давить на этих людей не возможно.

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

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

а, представил ситуацию. Обычно команда на стороне клиента усиленно гoвнокодит и валит все на аутсорсеров. Без заинтерисованого в качестве менеджмента можно не рыпатся, инфа 100%

Если нервы трепят — стоит поискать другое место. Если вы не паритесь — то и черт с ним- проект то не ваш личный. Пусть кодят как хотят...

Atlassian Crucible — как идеал. Всё остальное из моего опыта (google code review, redmine+плагин, tortoisediff) иначе, как костылями, назвать сложно. Но, если crucible не доступен — выкручивались, как могли. Ибо без него — просто ж%па. Во всех моих командах CR — обязательная составляющая процесса, не зависимо от методологии (waterfall, scrum, kanban etc).

Соответственно, процесс — практически дефолтный атлассиановский:
1) разработчик создаёт ревью (джуниор — из патча, миддл+ - из svn);
2) ревьювер(ы) просматривают, оставляют комментарии с рекомендациями/замечаниями;

3) когда все ревьюверы закончили — модератор выносит окончательное мнение, что должно быть изменено.

Если команда небольшая (до 5 чел) и все в одной технологии — то вся команда участвует в ревью. Если команда больше, или делится по технологиям (фронт/бэк/бд/и т.д.), то группы делятся соответственно.

Ревьювят как старшие молодых, так и джуниоры сеньоров — это обязательно.

Плюсы:
1) качество и читабельность кода вырастает очень быстро и держится на уровне;
2) новые люди в проект въезжают намного быстрее;

3) исчезает такой риск, как «в этом куске кода разбирается только Вася, а он болеет, а пока мы разберёмся, он выздоровеет...»

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

А вы crucible совместно с fisheye используете? Если нет, какие альтернативы? И, я так понимаю, если ривью создается из патча, нет возможности использовать веб-интерфейс (я имею ввиду тот же фишай)?

А вы crucible совместно с fisheye используете?
Они идут только в комплекте.
И, я так понимаю, если ривью создается из патча, нет возможности использовать веб-интерфейс (я имею ввиду тот же фишай)?
Почему? Патч заливается, автоматически устанавливаются привязки к файлам из хранилища — и точно также, со всеми плюшками и свистоперделками.

Они идут только в комплекте.

Разве? При попытке купить crucible, fisheye предлагается за отдельные деньги.

Я могу ошибаться. Просто я ни разу не видел Crucible отдельно от FishEye.

Мы сейчас тоже тест-драйвим crucible + fisheye, там просто схема такая — если у вас уже есть OnDemand JIRA то да, минимальные crucible + fisheye добавляются в комплекте за 10ку. А вот если вы хотите развернуть у себя и без JIRA, то тогда по десятке (ну или по $800 :) за каждый продукт

Оно по десятке и без джиры, насколько я помню. И 30$ (Jira+FE+Cru) — это смешная сумма, в обмен на которую получаешь относительный комфорт и кучу плюшек.

У нас 15 разработчиков и мы хотим развернуть у себя, а это $1600 — тоже смешно, но не так обхохочешься как за $30 :)

Создаёте 1 учётку модератора и 4 ревьювера. Креденшиалз расшариваете. В комменте к CR указываете, кто должен быть модератором, а кто ревьюверами. «Вам шашечки, или ехать?» © народ.

А кто такой модератор?

Кто такие ревьюверы? Их много? Сколько времени занимает такой процес?

А кто такой модератор?
Модератор — это тот же ревьювер, но который принимает решение по конкретному ревью. Ревьюверы только рекомендуют те или иные изменения.
Кто такие ревьюверы?
Коллеги, которые просматривают данное ревью. Пытаются найти в нём ошибки, отклонения от code conventions, да и просто разобраться в коде. В моих командах принято такое понимание: «Если ты отревьювил кусок кода, значит ты в нём разобрался, согласен, что он написан правильно, сможешь его поддерживать и готов отвечать, если там остались ошибки».
Сколько времени занимает такой процес?
Вообще зависит от размера команды. В командах до 5ти человек это занимает до получаса в день.
На разных проектах есть отступления, но из того, что использую последнее время больше склоняюсь к такому варианту:
1) VS 2010 + TFS 2010 + WinMerge(прикручен в студию для compare/merge действий)
2) Работа распределяется через TFS таски. Перед заливкой любых изменений, обязательно создается shelveset привязанный к таску и отдается на ревью.
3) Ревьювер забирает последние исходники, затем распаковывает shelveset у себя и проверяет:
3.а) Что проект билдится и запускается без ошибок (т.е. в шелв ничего не забыли включить)
3.b) Просматривает diff измененных файлов через WinMerge:
winmerge.org/..._inlinediff.png

3.c) Если есть замечания/вопросы — шелв отдается на исправление. Когда замечаний больше нет — дается добро на заливку содержимого шелва в репозиторий

Плюсы:
— большинство глупых ошибок и опечаток отлавливаются при ревью (хотя иногда все таки и ревьювер может их не заметить)
— Практически сводятся к нулю ситуации, когда у всех валится билд из-за того, что кто-то что-то забыл залить, или залил лишнее, или если изменения работали только у автора из-за особых изменениях в настройках среды (IIS, etc...) о которых он забыл рассказать.
— Более чистый код

— Как минимум еще 1 человек видел изменения по каждой задаче, т.е. bus factor уже больше 1 для любого куска проекта.

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

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

Используем Visual Studio 2010 + TFS 2010.
Перед чекином изменения кладутся на полку и создается таск на ревью. Ревьюер смотрит (построчное сравнение) и пишет замечания или апровит. Потом чекин линкуется на ревью таск.
Плюсы: все изменения проверяются и есть история кто чекинил и кто смотрел.

Сложности: тяжело проверять большие рефакторинги (например автоматический перенос и переименование методов). Такие лучше проводить отдельными изменениями. Сложно отслеживать изменения в больших файлах — лучше разбивать большие классы на несколько файлов.

В основном полагаемся на джентельменский набор:
1) Sonar
2) FindBugs как отдельный plugin для eclipse.

3) JDepend.

Проводим постоянно код ревью, не зависимо от того «синьёр» или «джун» или «лид» написал пару строк кода. Проект разбит по эриям, за каждую эрию есть ответственный. Именно человек ответсвенный за определенный кусок проекта, проводит код ревью если вдруг правится код в даной эрии.
Есть 2 вида правок.
1. Допиливание функционала(новый или баги).

2. Тулы для разработки.

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

Подписаться на комментарии