code review - надо или нет?

code review это миф. Как Дед Мороз, бог и адекватное правительство.

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

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

У нас для для джуниоров code review куратором обязателен (в основном). Еще в нескольких критичных проектах тимлиды все «ревьюят». Но основная разработка идет без этого.

Code review — это не «объяснение по строчкам».

Предложите свое определение :)

Еще - по строчкам никто ничего не объясняет (это какие-то шаманские практики). Кидаются коммиты в crucible и в комментах по коду обсуждается все сомнительное.

Наверное плохо выразился. Девелопер по собственной инициативе ничего не рассказывает. Но задача принимающего - полностью понять сделанное.

У меня происходит так - девелопер пушит код, я смотрю диффы. Если есть вопросы - созваниваемся по скайпу и быстренько решаем, что как. Иногда вопросов нет совсем. Иногда вплоть до каждой строчки. Как пойдет :)

У нас это происходит немножко по другому:

Периодически просматриваются коммиты от других программистов (в основном во время слияния веток). Если что-то в коде кажется странным или непонятным - пишешь в чат вопрос «Почему так».

Сейчас это происходит крайне редко, но постепенно пытаемся привить такую привычку.

У меня происходит так - девелопер пушит код, я смотрю диффы. Если есть вопросы - созваниваемся по скайпу и быстренько решаем, что как. Иногда вопросов нет совсем. Иногда вплоть до каждой строчки. Как пойдет :)

Отличный способ.

Серьезная разработка, имхо, без code review не должна жить. Еще введение формальных правил очень помогает (наподобие MISRA C/C++, можно их подмножество).

Или это с точки зрения бизнеса «разработки программных продуктов» не окупается?

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

Надо. Помогает найти ошибки в архитектуре и просто мелкие недочеты.

не окупается

За него еще и платить надо?

У меня происходит так - девелопер пушит код, я смотрю диффы. Если есть вопросы - созваниваемся по скайпу и быстренько решаем, что как.

ну и отлично, только не ефективно. прикрутите себе какую нибудь вебину для этого дела.

У меня происходит так - девелопер пушит код, я смотрю диффы. Если есть вопросы - созваниваемся по скайпу и быстренько решаем, что как. Иногда вопросов нет совсем. Иногда вплоть до каждой строчки. Как пойдет :)

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

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

Какую вебину? Гитхаб же :)

Да, каждый коммит проходит review 2 раза.

Почему два?

аутсорс. Местное ревью и ревью заказчика.

Нет, но недавно сам проверял коммиты у товарища, т.к. потребовалось в начале проекта договориться о том, что делать и чего не делать — хотел проверить, всё ли дополнительные правила он соблюдает и как оно пошло на реальном коде. Он тоже по своему желанию проверял мои коммиты.

В опенсурсе в QtCreator проверка каждого коммита обязательна. В паре патчиков для openmw тоже проверяли пулл реквесты.

Сейчас это происходит крайне редко, но постепенно пытаемся привить такую привычку.

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

А у вас на работе делается парный «обзор кода», как обязательный этап сдачи?

Да, если изменения более-менее серьезные. Мелкие правки обычно не ревьюятся.

Я сейчас использую gitk, в целом не плохо.

Надо. Это ещё один этап отлова багов и проверки соблюдения соглашений по качеству кода. Оно также даёт хорошее представления одним участникам проекта о том, чем занимаются другие.

с точки зрения бизнеса «разработки программных продуктов» для обеспечения качества в процессе «разработки программных продуктов» code review необходим. Да он не нравится разработчикам, да на него тратится драгоценное время, да в большинстве случаев на инспекции не находится ни одного бага и пропускаются глупые косяки. Но если в вашей профессиональной команде установлен «процесс разработки» для обеспечения качества, то этот процесс не мыслим без обзора кода.

А у вас на работе делается парный «обзор кода», как обязательный этап сдачи?

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

Случаются сеансы парного программирования. Иногда надо разобраться в древнем легаси и что-то починить - здесь 2 светлые головы лучше одной светлой.

Еще одна проблема - уровень исполнителей. Мой код очень тяжело дается коллегам, не доросли еще до nightmare level. Поэтому мои проекты автономны, и код вижу только я.

конечно надо. Как и CI.

Или это с точки зрения бизнеса «разработки программных продуктов» не окупается?

Окупается. Стадо студней + 1 человек с плеткой.

читаем код друг друга, ржём всем офисом (с) =)

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

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

есть идеи, как внедрить/заставить? ;)

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

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