Понимаю что такая тема может быть оффтопом на ДОУ, но все-таки отвлеку сообщество от обсуждения ЗП и судьбы очередного джуна.
Интересует как у вас в организации/команде проводится данный процесс: какие тулы используете, насколько «обязательные» ревью, сколько времени занимает и тд.
Какие плюсы/минусы получили? Какие сложности были?
Если таки не внедрили, то почему?
Интересует личный опыт, теоретические статьи я читал :)
github pull request? И комменты.
У нас был обязательный как минимум один peer review от человека, которого ты считаешь компетентным в вопросе. Если никого, то тогда просто больше глаз
Я один тут, как нормальный человек из Idea делаю код ревью?
Я один тут, как нормальный человек из Idea делаю код ревью?А замечания куда пишете?
Кричу матом на всю комнату.
p.s. А если серьезно, то записываю в текстовый файл.
p.s. А если серьезно, то записываю в текстовый файл.А потом файл выкладываете на тот же ФТП (или на тот же флопи) кде лежит код :)
У меня был таск на Nokia online проекте — делать code review для команды индусских программеров. ржал много :)
Для понимания процесса code review полезно помнить, что цель и результат успешного сабжа — готовность ревьюера поставить подпись под каждой строкой кода.
что цель и результат успешного сабжа — готовность ревьюера поставить подпись под каждой строкой кода.
Ну, цель таки — качественный код, а то что вы описали — это, скорее, сайд эффект.
Коментар порушує правила спільноти і видалений модераторами.
По моему скромному мнению лучше написать стандарт кодирования (выделить чьи-то человеко-часы для этого). И после обсуждения и принятия командой — пусть каждый сам пишет код по описанным стандартам.
Коде ревью — это попытка на «народном виче» принять решение по каждому пустяку, вместо готового закона.
Коде ревью — это попытка на «народном виче» принять решение по каждому пустяку, вместо готового закона.
Неправильно. Садись, два.
Вообщем ревьюить кодинг стайл это млин тоже входит в ревью кода, но млин никак уж его не ограничивает. Да, лично для себя нашел удобным делать синк в начале рабочего дня и смотреть кто чего накодил. Очень помогает. А вот код ревью как выделенный процессный таск меня немножко раздражает, именно не по дифу в динамике, а в статике. Тебе бухают тонну сырца и говорят, вот те пару дней-ночей, выдай нам все, что ты думаешь. Вот это обычно ведет к галочном варианту, единственное если код нормально задокументирован на всех уровнях то тогда может быть ок (комменты по коду, диаграммы, описалово хдд-сдд), то тогда можно, а так фигня какая то (мало пользы наблюдал). Мое мнение и опыт, не судите строго, ну не люблю я эти все модные процессы. Иногда за ними и полезной работы не видно, и проект замер, и сам вообщем то не растешь профессионально, зато бумажка на бумажке и у всех пятая точка прикрыта.
какие тулы используете
Текстовый редактор для написания отчета.
насколько «обязательные» ревью
Незнаю — никогда не отказывался от такого полезного процесса.
сколько времени занимает
Часа два-три.
Какие плюсы/минусы получили?
Другие тебе указывают на твои слабые места, и говорят, как их подтягивать.
phabricator.org никто не пробовал в живых проектах?
git rebase —onto <original_review_revision> && git push # перетянуть эти новые комменты и коммиты назад и запушить
плюс — не нужно никаких специальных тулзовминусы — в crucible ревью комментарии от всех участников появляются в реальном времени, может быть удобнее, особенно для распределенных проектов.
попробуйте вот это
code.google.com/p/gerrit
пробовали разное. в данном случае описывался простейший способ с
плюс — не нужно никаких специальных тулзовС gerrit-ом сам не работал, но соседние комманды жаловались на очень негибкий workflow.
Код-ревью — это вообще очень сильная проверка/встряска для команды. Очень сложно достичь момента, когда критика воспринимается не лично, а с пониманием, что это во благо. Сложно правильно делать ревью, тоже нужна деликатность, т.к. каждый любит свой код и не хочет признавать, что в нём, что-то не так. Тем не менее, такую встряску я нахожу полезной, выводит отношения на новый уровень понимания и доверия.
т.к. каждый любит свой код и не хочет признавать, что в нём, что-то не так.
Когда в команде у каждого свой стиль кода — это плохо. А когда есть Code Conventions — тут и критики особой нет.
Когда в команде у каждого свой стиль кода — это плохоГлавное — это чтобы код читался (скажем, отсутствие
если у вас основные претензии — только к разношерстным стилям, то это круто.
У кого-нибудь есть опыт работы с Google Code Reviews/Rietveld? Особенно интересно узнать, пробовал ли кто-нибудь развернуть его на локальной машине.
Пробовали. Впечатление — «костыль».
Сборка проектов — 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
Даже было начал писать подобное решение, но пока подзабил.
Коментар порушує правила спільноти і видалений модераторами.
crucible
> насколько «обязательные» ревьюесли проект небольшой, то вся команда делает ревью обязательно, если большой то тимлид, обязательно остальные «крайне желательно».
> сколько времени занимаетустановка на чтобы занимало <=0.5 часа в день. В подавляющем большинстве случаев этого хватает.
> какие плюсы/минусы получили?повышение качества кода, распространение знаний о уже существующих методах/подходах/реализациях (в большом проекте, особенно том который мы не начинали с 0, кодревью может уберечь от появления велосипедов по незнанию того что «это где-то уже сделано кем-то и так-то»)
Как впечатления от crusible`а?
В целом очень положительные!
Хотя мелочи конечно есть всякие. Как по мне не хватает наглядности при работе с комментариями. Например комментарий можно пометить, что «вот тут баг», указать приоритет и тип. Но как потом посмотреть список моих комментариев с пометкой «дефект», посмотреть ответил ли на них автор ревью (у нас принято потом отписаться в стиле «пофикшено»). Или просто посмотреть все комментарии для всех ревью которые еще не summarize-ны, там иногда развивается дискуссия а через почтовые уведомления следить за ней неудобно... В общем лично мне чуть не хватает обратной связи.
мы в jira ведем обратную связь, ревью в крусибле — вторично и довесеок к тикету
Поддержу Нестора. Модератор просматривает все комменты и выносит решение. Решение пишется в ревью и может дублироваться в джиру.
Хорошо, правда не всегда удается нормально изменения запихнуть туда, но это терпимо. Удобный инструмент.
правда не всегда удается нормально изменения запихнуть туда
А какие проблемы могут возникнуть? Какого рода?
У нас были проблемы с патч-файлами в случае сложного дерева веток и меток. Решилось просто — стали делать патчи в эклипсе на уровне проекта.
Одну уже сказали, вторая — невозможность нормально скопипастить код при ревью.
Объясните, пожалуйста, подробнее. Не могу понять проблему.
crucible
это просто «Das ist fantastisch!» по сравнению с тем что я ещё повидал. Опять же инеграция в джиру и прочие ихние проекты. must have.
теоретически обязательно :) , реально по разным причинам может быть отложено (очень редко)
Наверное, наш подход весьма распространен. Ревью проводится без тулов: просто садимся за вдвоем с автором и ревьюим коммиты.
Ревьюим не все. Поводы для ревью:— кому-то было скучно, и он решил провести ревью — такое тоже случается
Занимает в зависимости от задачи — иногда десять минут, иногда два-три часа. Замечания по ревью оформляются в виде мейла и отправляются в рассылку проекта. Заголовок письма имеет определенный формат, поэтому такие письма легко отфильтровать. Вики пока не заводим. Вроде бы и надо, но как показывает практика, вики труднее поддерживать в актуальном состоянии, да и читают их редко.
Ревью может провести любой участник проекта, но обычно ревьюит кто-то с опытом.
Минусы и проблемы:— т.к. ревьюятся не все коммиты, косяки все-таки проникают в код. Не могу сказать, что это большая проблема. Мы используем жесткий чекстайл и FindBugs, так что значительная доля косяков не доходит до коммитов.
Используем переделанный под свои нужнды CollabNet TeamForge.
Для внесения изменений в core код, необходимо ревью минимум двух человек. Фактически имеется возможность любому подписаться на какое-нибудь событие изменения любого кода в системе самому и учавствовать в ревью, если есть необходимость. Патчи рассылаются как по почте, так и складируется на личной страничке, где можно либо согласиться, либо исправить код, либо высказаться против и т.д. Потом всё автоматически комитится в svn. Без ревью не будет комита.
Для нового кода обычно используются один, максимум два человека, которые могут сделать code review, но обычно до релиза никто код не смотрит.
Времени занимает немного, т.к. много людей, главное не тянуть одеяло на себя и не учавствовать во всех ревью :)
Сложностей не знаю, пришёл на готовое :) Минусов я не нашёл. Из несомненных плюсов, ощущение полного контроля над кодом, очень тяжело кому-то «запороть» или внести критические изменения в ответственные участки кода.
Минусов нет.
Разработка проекта ведется вечерами (каждый у себя дома) и на выходных (собираемся вместе, пеир программинг и обсуждение проблем/решений за чашечкой чая/кофе)
При разработке придерживаемся следующих правил:— выкат на лайв.
2. Тесты пишем только на ЛОГИКУ и на баг для фикса. Тоесть перед фиксом баги пишется тест, который эту багу выявляет и в последствии этот тест должен пройти.
Учитывая специфику работы (вечера+выходные) иногда выкат задачи из-за ревью откладывается на день-два. Но профит в том, что транк всегда чист и стабилен (куча удовольствия для разработчиков которые любят свое дело) в последствии обновление сервиса происходит в течении 2 минут (сейчас немножко доьше в связи с переходом в ажур).
После 2 недель обкатки такого, нового для нас, процеса мы признали его идеальным для нас.
Тоесть по сути вместо дополнительных тулзовин добавили статус «код ревью» к задаче. Соответвенно ревьювер просматривает и пронюхивает код. Оставьляя свои впечатления в коментариях к задаче.
По моим ощущениям- слабенькая замена настоящему XP с парным програмированием. На безрыбье покатит.
для ревью уже после комита исползуем crucible
Почему не использовать ревью и для pre-commit? Похоже, в вашем случае оно бы помогло решить и проблему
Сложности — некторых задротов не любят звать на ревью.
Да, и нельзя позиционировать CR, как замену XP в общем и парному программированию в частности. Это просто разные инструменты.
Крусибл пока не постоянно используется, прекомитт ревью пока делаю только я. Нет требоваия использовать — ленятся и динамят.
Команда специфическая, основной источник гемороя — синийор девелоперы по дружбе друг дружку по бырику отревьювившие. Дать им по шапке достаточно сильно что бы перестали я пока не могу )
Попробуйте распространить эту практику на тех, кого возможно. И постараться добиться ситуации, когда синьоры будут делать больше ошибок, чем джуниоры ;)
работаем с гитом, исползуем Gitlab для код ревью. В Гитлабе делаем запрос на мердж с дев веткой, потом просматриваем дифф. Помогает проследить за архитектурными решениями, так же иногда подсказать лучший способ сделать что-то.
Минусов нет, потраченое время одного из члена комманды помогает в поиске истины, при просмотре чужого кода можно направить человека на путь истиный, либо же самому чему-то научиться.
Помогает следить за одним кодстайлом в проекте.
В будущем планируем Jenkins + Phing — тесты, веб морда, код стайл.
Почти без ревью, тулов никаких, организации нет, будет ревью если сам попросишь, естественно таки образом совершенно не ревьюится г о в н о(спасибо за фильтр «ужасных» слов)код. Если что-то и делается то от этого не много толку, всё равно куча людей даже не проверяет компилируются ли их код и заливает как есть. Так жить нельзя, а давить на этих людей не возможно.
в бытность отведственной по бараку обизяной сильно крепил за такие вещи — помагало, вообще полезно. Если у вас обер капо это не наладит — налаживайте сами — а то будет становится все хуже....
Сложно если единственный способ коммуникации это письма, которые никак не убеждают, а вышестоящие над не добросовестными товарищами глухи к абсолютно любым просьбам относительно изменения процесса, особенно когда отсутствие ревью лишь малая из бед.
Если нервы трепят — стоит поискать другое место. Если вы не паритесь — то и черт с ним- проект то не ваш личный. Пусть кодят как хотят...
Atlassian Crucible — как идеал. Всё остальное из моего опыта (google code review, redmine+плагин, tortoisediff) иначе, как костылями, назвать сложно. Но, если crucible не доступен — выкручивались, как могли. Ибо без него — просто ж%па. Во всех моих командах CR — обязательная составляющая процесса, не зависимо от методологии (waterfall, scrum, kanban etc).
Соответственно, процесс — практически дефолтный атлассиановский:3) когда все ревьюверы закончили — модератор выносит окончательное мнение, что должно быть изменено.
Если команда небольшая (до 5 чел) и все в одной технологии — то вся команда участвует в ревью. Если команда больше, или делится по технологиям (фронт/бэк/бд/и т.д.), то группы делятся соответственно.Ревьювят как старшие молодых, так и джуниоры сеньоров — это обязательно.
Плюсы: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ти человек это занимает до получаса в день.
3.c) Если есть замечания/вопросы — шелв отдается на исправление. Когда замечаний больше нет — дается добро на заливку содержимого шелва в репозиторий
Плюсы:— Как минимум еще 1 человек видел изменения по каждой задаче, т.е. bus factor уже больше 1 для любого куска проекта.
Минусы:— требуется время и определенные усилия лида, чтобы ввести подобную практику в команде, особенно если раньше ревью не проводили.
TFS...
Сложности: тяжело проверять большие рефакторинги (например автоматический перенос и переименование методов). Такие лучше проводить отдельными изменениями. Сложно отслеживать изменения в больших файлах — лучше разбивать большие классы на несколько файлов.
3) JDepend.
2. Тулы для разработки.
Для вторых привлекается архитектор, который уже будет решать целесообразность этой фичи(или тулы). Код ревью занимает от 5 до 45 минут, в зависимости от количества поправленных файлов и внятности обьяснений «зачем» это было сделано и «как» оно работает. После чего ревьер аппрувит код или говорит доработать. После доработки опять идет код ревью. На этапе заливки в репозиторий указывается кто ревьюер и кто делал пре чек ин(бычно делает мануал тестер).
Igor Petruk
Ivan Pomidorov
Ivan Pomidorov
Nikolay Karakulov
KC
Alexandr Gavriluk
Mike Gorchak
chronoman
Andrii Muzychuk
proger
SemiCoder
Mykola Gurov
Vitaliy Ryumshyn
Mykola Gurov
Anton Vynogradenko
Agnislav Onufriichuk
св Антоний
Євген Козлов
Andrey Dankevich
Kirill Zotin
Iaroslav Voitovych
Anton Vynogradenko
Valeriy Zamaraiev
Егор Талдыкин
Maxim Bregeda
Егор Талдыкин
Нестор Иванович
Alexey Grigorev
Dmytro Butakov
Diversity Equity and Inclusion
Misha
Андрей
Mike Gorchak
Bohdan Chechyn
Pavel Maliavkin
Нестор Иванович
Pavlo Voznenko
Pavlo Voznenko
Agnislav Onufriichuk
Pavel Maliavkin
Andrusenko Dmytro
Beaver Green
Leon KPI
73 коментарі
Додати коментар Підписатись на коментаріВідписатись від коментарів