Привет всем,
кто-нибудь использует такую процедуру как Code review? Как это происходит? вы реально выделяете час времени в неделю, садитесь с командой и начинаете стебаться друг над другом???
Хотелось бы услышать мнение тех, кто переодически учавствует в подобном мероприятии. Есть ли смысл проводить Codу review? Ведь когда девелоперы работают на проекте, они так и так смотрят код друг друга.

Здравствуйте, debugx, Вы писали:

D>Привет всем,
D>кто-нибудь использует такую процедуру как Code review?
У меня на одном из прошлых проектах такое было. Надо сказать, не бесполезно.

D>Как это происходит? вы реально выделяете час времени в неделю, садитесь с командой и начинаете стебаться друг над другом???
На том проекте — да, именно так.

D>Хотелось бы услышать мнение тех, кто переодически учавствует в подобном мероприятии. Есть ли смысл проводить Codу review?
Определённо, да. Хотя и не обязательно именно в таком виде

D>Ведь когда девелоперы работают на проекте, они так и так смотрят код друг друга.
Тут есть два ответа:
1. А зачем им смотреть код друг друга? Если проект маленький — то да, все видят всё. А если человек на 50?
2. А если они-таки смотрят. то это и есть одна из форм code review

Здравствуйте, debugx, Вы писали:

D>кто-нибудь использует такую процедуру как Code review?
Регулярно. Код не чекинется без этого.

D>Как это происходит?
Беруться измененные файлы до и после изменений и высылается на инспекцию. Если изменения существенны желательно еще иметь описание того что было сделанно. Можно использовать различные тулзы для этого.

D>вы реально выделяете час времени в неделю, садитесь с командой
Не час, а столько, сколько нужно. Каак только реализация закончена код отправляется на проверку.

D>и начинаете стебаться друг над другом???
Это же не детский сад. Если есть проблемы в коде они указываются.


D>Есть ли смысл проводить Codу review? Ведь когда девелоперы работают на проекте, они так и так смотрят код друг друга.
Есть. Проекты большие бывают и делают их разные люди.

Здравствуйте, debugx, Вы писали:

D>Привет всем,
D>кто-нибудь использует такую процедуру как Code review? Как это происходит? вы реально выделяете час времени в неделю, садитесь с командой и начинаете стебаться друг над другом???
после выполнения задачи другой человек (не команда, выбирается соответствующего уровня) проверяет, правильно ли реализовано то что требовалось, в достаточном ли объеме, иногда даже не в лишнем ли объеме. проверяется как код (соответствие стандарту, соглашениям, корректность, отсутствие багов), так и логика.
никакого стёба, если все в порядке, ревью прошел, если что-то не так, отправляется на доработку, с объяснением почему, что доделать.

D>Хотелось бы услышать мнение тех, кто переодически учавствует в подобном мероприятии. Есть ли смысл проводить Codу review? Ведь когда девелоперы работают на проекте, они так и так смотрят код друг друга.
это формальная проверка правильности реализации задачи. если у тебя программа оперирует миллионами где-то или должна работать постоянно и часто перегружать невозможно — такие проверки нужны.

Здравствуйте, debugx, Вы писали:

D>кто-нибудь использует такую процедуру как Code review? Как это происходит? вы реально выделяете час времени в неделю, садитесь с командой и начинаете стебаться друг над другом???

3 дня назад мне делали. общий обьем — что-то около 3к строк (всего).
человек затратил около 2-х недель, что бы вычитать весь мой код.
сидели 2 часа в компании с еще одним, в митингруме с проектором — отвечал на вопросы. вроде отбился.
стебатся не стебались. рисовал на доске варианты рампределения нагрузки и оптимизации запросов, работу индексов и схему данных.

...только отказался отвечать на 1 вопрос. меня попросили "коротенько" рассказать про аналитические функции оракла.
потупил 2 минуты, и попросил почитать доку.

D>Хотелось бы услышать мнение тех, кто переодически учавствует в подобном мероприятии. Есть ли смысл проводить Codу review? Ведь когда девелоперы работают на проекте, они так и так смотрят код друг друга.

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

Здравствуйте, debugx, Вы писали:

D>Привет всем,
D>кто-нибудь использует такую процедуру как Code review? Как это происходит? вы реально выделяете час времени в неделю, садитесь с командой и начинаете стебаться друг над другом???
D>Хотелось бы услышать мнение тех, кто переодически учавствует в подобном мероприятии. Есть ли смысл проводить Codу review? Ведь когда девелоперы работают на проекте, они так и так смотрят код друг друга.

Перед тем, как заливать код в TFS проводится ревью кода одним из сениор девелоперов. Важно, что код сениоров тоже подвергается ревью. Формально в хранилище кода TFS вообще не попадает код, который не был проинспектирован. Впрочем, понятно, что мелкие изменения типа переименования внутренней переменной метода никто не проверяет.
Чтобы ускорить процесс ревью активно пользуемся функциями shelve/unshelve. Таким образом ревьюверу даже не надо отрывать свой зад от кресла, чтобы отревьювить чужой код.

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

SE>Формально в хранилище кода TFS вообще не попадает код, который не был проинспектирован.
Жестоко. Бранчеваться не пробовали?

Здравствуйте, VGn, Вы писали:

SE>>Формально в хранилище кода TFS вообще не попадает код, который не был проинспектирован.
VGn>Жестоко. Бранчеваться не пробовали?
да чет жестковато. некий вариант парного программирования.
интересно. сам в таком режиме пока не работал. ...и чет не хочу. во

Здравствуйте, VGn, Вы писали:

SE>>Формально в хранилище кода TFS вообще не попадает код, который не был проинспектирован.
VGn>Жестоко. Бранчеваться не пробовали?

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

Здравствуйте, bastrakov, Вы писали:

SE>>>Формально в хранилище кода TFS вообще не попадает код, который не был проинспектирован.
VGn>>Жестоко. Бранчеваться не пробовали?
B>да чет жестковато. некий вариант парного программирования.
B>интересно. сам в таком режиме пока не работал. ...и чет не хочу. во

Инспекция кода занимает у меня не более получаса в день, да и то, не каждый день.
А вот если б я на каждый бзик бранч создавал, представляю сколько б я возился c тормознутым TFS туда сюда сливая и создавая различные ветки и постоянно между ними переключаясь.
Ведь такого, чтоб девелопер крутил только свою гайку не бывает. Часто надо помогать другим их гайки докручивать. Или свои гайки кому-то отдавать. А гаек-то ого-го.

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

SE>Как по мне, бранчи идеологически предназначены для крупных частей проекта, которые по тем или иным причинам должны разрабатываться независимо. Т.е. бранчи, имхо, не для того, чтоб на каждый чих их заводить.
SE>А вот на каждый день очень подходят шелвсеты, которые именно и предназначены для временного хранения слепков кода во время разработки до тех пор, пока разработчик не решит свою часть работы залить в TFS. Их же (шелвсеты) очень удобно использовать и для ревью.
Идеалогия — это конечно хорошо. Но версионность, она просто тупо удобна при разработке независимо от командных целей. (у меня на ноуте локально стоит SVN. Активно пользуюсь. Ни у кого разрешения на чекин не спрашиваю )
Вообще везде, где мне приходилось работать, более удобными считались короткие чекины. Притом их банально проще мержить. Непрерывная интеграция и всё такое.
А для стабилизации как раз используются бранчи.
Шелвсеты — личный велосипед Мелкософта. У других средств я аналогии им не знаю и, честно говоря, не очень и пользуюсь. На мой вкус они сильно фрагментируют код и удлиняют периоды интеграции, внося бардак в контроль конфигураций. Хотя конечно всё зависит от процесса.

Здравствуйте, SE, Вы писали:

SE>>>>Формально в хранилище кода TFS вообще не попадает код, который не был проинспектирован.
VGn>>>Жестоко. Бранчеваться не пробовали?
B>>да чет жестковато. некий вариант парного программирования.
B>>интересно. сам в таком режиме пока не работал. ...и чет не хочу. во

SE>Инспекция кода занимает у меня не более получаса в день, да и то, не каждый день.

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

Здравствуйте, bastrakov, Вы писали:


D>>кто-нибудь использует такую процедуру как Code review? Как это происходит? вы реально выделяете час времени в неделю, садитесь с командой и начинаете стебаться друг над другом???

B>3 дня назад мне делали. общий обьем — что-то около 3к строк (всего).
B>человек затратил около 2-х недель, что бы вычитать весь мой код.
B>сидели 2 часа в компании с еще одним, в митингруме с проектором — отвечал на вопросы. вроде отбился.

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

3к строк будет примерно 15 часов чистого времени. При условии 4-х часов чистого времени в день (полная загрузка) имеем дня 4, вообще-то. То есть, кодревью выполнялось скорее медленно. Если ревьювер при этом еще и не нашел в коде багов (критерий эффективности ревью, ваще на нем ошибки положено ловить) — это отрицательная работа. От такого code review пользы немного.

Медленно и неэффективно оно может быть по разным причинам. Например:
1) Человек, кто проводит код ревью, не проводил ранее дизайн ревью этого же кода. Лечится либо kick-off meeting перед началом ревью (что считается необходимым для экспертизы материала такого объема), где автор объясняет, что тут ваще происходит, и как оно ваще работает. Либо, что гораздо эффективнее во всех смыслах, введением отдельного дизайн-ревью, где автор докладывает, как он собирается решать задачу, до того, как пишет основную массу кода. Для дизайн ревью в простейшем случае сойдет устный доклад у маркерной доски. И те же люди, кто проводил дизайн ревью, потом выполняют код ревью.
2) Человек вообще не специалист в данной теме — маловероятно, что он найдет какие-либо ошибки кроме пунктуации, эстетики, и соблюдения стандарта кодирования. Подобные ошибки, к слову, ловятся в темпе побыстрее чем 200 строк в час.
3) Давайть на ревью такие большие фрагменты кода — фашызм. Надо стараться бить на части. Опять же, проводя до этого дизайн ревью, чтобы люди были в курсе.

Вообще, дизайн ревью необходимы для того, чтобы сделать code review более эффективными на практике. Почему:
1) Они снижают затраты на code review, и совокупные затраты на все review.
2) Ошибка, которая может быть обнаруженная на дизайн ревью, в исправлении почти бесплатна, и является напротив — наиболее дорогой ошибкой, будучи найденной на код ревью.
3) Надо учесть социальный фактор. Бывает так, что на кодревью выясняется, что весь подход к решению задачи неправильный, и делать по хорошему надо не так. Однако, автор уже написал дохрена кода! Во-первых, ревьюверы чувствуют себя виноватыми, и им тяжело сказать человеку, что надо все выбросить, и переделать. Во-вторых, человеку самому грустно и обидно это делать. Поэтому, такой код имеет все шансы пройти кодревью, и таки оказаться в репозитории. Если не проводить design review, подобная ситуация будет возникать регулярно.
4) Design review позволяют привить культуру предварительного проектирования в масштабе компании, и являются, кроме того, единственным способом как-то контроллировать закрытие задачи "проектирование".

D>вы реально выделяете час времени в неделю,
Унас можно потратить максимум 10% времени. Т.е. 4 часа в неделю. Обычно это достаточно. Щас посмотрел по отчётам, примерно 2ч в неделю реально тратится (на ревьюв чужого кода и исправления в своём после ревьюва)

D>садитесь с командой и начинаете стебаться друг над другом???
Собрать всех вместе это наверное малореально. Та и базара будет больше. Каждый делает ревьюв когда есть время. Я обычно делаю ревьювы между задачами. Это помогает немного переключится. Даже можно сказать, даю отдохнуть мозгам. Единственное правило, ревьювы не должны устаревать. Их надо постоянно делать, нельзя сделать ревьюв раз в месяц. Иначе после этого ревьюва уже могли быть комиты и ревьюв не актуален (мы разрешаем комитить, ревьювим после).
Нельзя ревьювить больше 3х часов. Вобщем много всяких мелочей.

D>Ведь когда девелоперы работают на проекте, они так и так смотрят код друг друга.
Как уже писали, они видят код, только если проект маленький. А так уних нет времени смотреть. Уних есть контракты между модулями. Их палкой не заставишь сидеть рассматривать что там внутри. Своих тасков хватает.

Но самое главное в ревьюве это тулза. Программисты ленивые. Ревьюв должен создаваться "одной кнопкой" (как и билд). Пару лет назад я начинал внедрять ревьюв. Пробывал смотреть дифы и писать письма\баги. Это столько оверхеда, что быстро сдают нервы. Самые оптимальные Crucible и Code Collaborator. Уних разные воркфлоу. Мне больше нравится Code Collaborator (хоть там и проблема с лицензией).
Есть вопрос. Кто ещё знает достойные codereview tool? Обязательно чтоб web based. Сервер можно усебя поднять. Бесплатный или лекарство можно найти.

Здравствуйте, DimitrySTD, Вы писали:

DST>Есть вопрос. Кто ещё знает достойные codereview tool? Обязательно чтоб web based. Сервер можно усебя поднять. Бесплатный или лекарство можно найти.

http://www.review-board.org/ смотрели?

DST>>Есть вопрос. Кто ещё знает достойные codereview tool? Обязательно чтоб web based. Сервер можно усебя поднять. Бесплатный или лекарство можно найти.
L>http://www.review-board.org/ смотрели?
Красивый интерфейс. Но суть примерно как в crucible. Можно написать замечания к diff. Но потом тяжело контролировать, что они исправлены. В колабораторе я могу создать коментраии\баги. После исправлений, мои коменты\баги мапятся на новый diff. Т.е. я вижу исправления и текст бага. Закрываю если надо, открываю новый (да-да, такое бывает. Исправляют и делают новый баг в исправлении). Ревьюв сам закроется когда багов нет или я их все закрыл.
Такая схема удобна когда ревьювишь несколько проектов. Когда программисты новички и делают много ошибок. Пока такой воркфлоу только в колабораторе видел

Здравствуйте, DimitrySTD, Вы писали:

DST>Но самое главное в ревьюве это тулза. Программисты ленивые. Ревьюв должен создаваться "одной кнопкой" (как и билд). Пару лет назад я начинал внедрять ревьюв. Пробывал смотреть дифы и писать письма\баги. Это столько оверхеда, что быстро сдают нервы. Самые оптимальные Crucible и Code Collaborator. Уних разные воркфлоу. Мне больше нравится Code Collaborator (хоть там и проблема с лицензией).
DST>Есть вопрос. Кто ещё знает достойные codereview tool? Обязательно чтоб web based. Сервер можно усебя поднять. Бесплатный или лекарство можно найти.

Code Collaborator — великолепный тул. Дорогой, да. Дешевая версия — от 200 USD за место. Кроме того, если брать плавающие лицензии — то будет дешевле, чем количество мест. Для данного тула — плавающие лицензии это скорее правильно. Так как железобетонно ограничивают время, потраченное на code review.

Вероятно, это единственный тул из всех платных процессных тулов, который имеет прямой и очевидный эффект на продуктивность и качество. Этот тул окупается. Надо обязательно брать. Musthave.

Собственно, спасибо за ссылку на такой классный, годный тул. Такие тулы — редкость.

G>Code Collaborator — великолепный тул. Дорогой, да. Дешевая версия — от 200 USD за место. Кроме того, если брать плавающие лицензии — то будет дешевле, чем количество мест. Для данного тула — плавающие лицензии это скорее правильно. Так как железобетонно ограничивают время, потраченное на code review.
Да, цена очень кусучая. Увы мы так и не купили. И наверное не смогли бы пользоваться, если бы не глюк в ихнем триале (сервер ccollab_server_2_1_725_windows.exe). Он работает бесконечно. Каждый день пишет что завтра закончится. Мы ничё не крутили, оно само . Вобщем так и живём.
Конечно в новой версии много вкусностей. Но лекарство нереально найти ни для новой ни для старой. Хотя скоро прижмёт. Старый клиент не умеет работать с svn 1.5 Так что пока не апгрейдим репозитарий.

Здравствуйте, DimitrySTD, Вы писали:

G>>Code Collaborator — великолепный тул. Дорогой, да. Дешевая версия — от 200 USD за место. Кроме того, если брать плавающие лицензии — то будет дешевле, чем количество мест. Для данного тула — плавающие лицензии это скорее правильно. Так как железобетонно ограничивают время, потраченное на code review.
DST>Да, цена очень кусучая. Увы мы так и не купили. И наверное не смогли бы пользоваться, если бы не глюк в ихнем триале (сервер ccollab_server_2_1_725_windows.exe). Он работает бесконечно. Каждый день пишет что завтра закончится. Мы ничё не крутили, оно само . Вобщем так и живём.
DST>Конечно в новой версии много вкусностей. Но лекарство нереально найти ни для новой ни для старой. Хотя скоро прижмёт. Старый клиент не умеет работать с svn 1.5 Так что пока не апгрейдим репозитарий.
Хех, а не вы ли лично полтора года назад просили ключ на руборде? Обрадую вас: вам ответили (и довольно давно).

СУВ, Aikin

P.S. ccollab_server_4_0_856_windows.exe, выкачанный только сегодня, с радостью съел ключик