Существует мнение в «народе», что на территории exUSSR очень плохо обстоят дела с инженерными практиками разработки, в частности с code-review. А так ли это?
Совершенно согласен, никак. Но, например, многие из моих знакомых-разработчиков не проводят ревизий кода, постоянно ссылаясь на давление по срокам, постоянный вброс фич и так далее. Хотя где логика — не ясно. Ведь выдача кода «на гора» без code-review только затянет проект из-за потенциальных багов в нем.
То что CR не панацея, верно. Но грустно смотреть на некоторые компании, которые пытаются давить на тестирование, чтобы те «лучше тестировали», но не хотят приходить к таким вещам как code-review или unit-тесты. Хотя мы уже уходим от темы…
— Все разработчики собираются, допустим, раз в месяц, выделяют несколько дней и досконально изучают весь код, обсуждая (и исправляя) спорные места.
— Архитектор держит в голове всю структуру проекта и следит за каждым коммитом, пинная разработчика если тот коммитает что-то эдакое. Тоже code-review в каком-то смысле :)
Используем Review Board.
Это не совсем полноценное ревью, таким образом отслеживаются ошибки связанные, в большей степени, с невнимательностью, чем глобально-архитектурные.
Если у приложения хоть сколь-нибудь длительный срок поддержки (например, полгода и больше), то без качественного кода потом ой как сложно приходится.
Понятно, что не все заказчики (внешние или внутренние) готовы платить за качество кода, но это больше от непонимания. Если заказчик выбирает быстро и дешево, нужно несколько раз убедиться, что он понимает свой выбор, перед тем как соглашаться на проект. Или не соглашаться :)
Не всё так страшно.
Мне пару раз в год приходится ковыряться в старых (лет восемь) приложениях на PHP — новую функциональность добавить или внезапно проявившийся баг исправить.
Там ужас-ужас-ужас в стиле php3, обращения к бд чередуются выводом html, get-параметры используются как переменные и т.п. Но приложения все боевые, используются многими людьми каждый день, и вмешательства требуют крайне редко. Причём это не какой-нибудь сайт, а «корпоративные» приложения.
Так что качество кода — не главное.
Время от времени тоже приходится заниматься своим древним проектом на C++ (99-й год вроде). Там тоже все через одно место написано, но со своим кодом работать, конечно, легче.
Проводим, но редко, когда возникают проблемы, и не всего кода.
Надо бы регулярно по идее, штука очень полезная. Все осложняется roadmap'ом и огромным количеством кода и подпроектов.
Без code review нельзя сделать очень критичный проект 24x7, кде каждая минута простоя это более чем 1 000 000 евро. Если код не знаем более чем один человек — это в большенстве случаев выброшенный код. Офигенно рулит техник pair programing для подобных проектов.
Юрий, тут, скорее всего, многое зависит от зрелости команды. Ни в коем случае не камень в ваш огород, но у нас верификация и валидация — это непрерывный каждодневный процесс, «вшитый» в нашу методику разработки. Поэтому к окончанию итерации мы уверены, что мы делаем правильные вещи и делаем эти вещи правильно, а вот как мы их делаем на уровне кода — смотрим, оцениваем, да и просто вся команда проекта знакомится с кодом — вдруг придётся с ним работать.
Тогда получается, что мы с вами используем ревью совсем для разных целей. У нас это
— непрерывный контроль качества кода в процессе реализации,
— дополнение к не всегда хорошо поставленной верификации,
— помощь разработчикам в освоении новых для них частей системы.
У вас же это скорее некая метрика. Тоже интересный подход :)
У нас каждый pull request проходит code review одним или двумя разработчиками (зависит от сложности), без этого код не проходит в основную ветку разработки. То есть обычно разработчик за день читает один-два чужих pull-request'а.
Из бесплатных неплох Review Board, рассчитанный в первую очередь на precommit review. Из коммерческих, кроме CodeCollaborator, стоит посмотреть на Crucible от Atlassian.
Пришлось недавно пообщаться с Review Board — никакого сравнения с CodeCollaborator.
Начиная с того, что «интеграция» с VCS заключается в том, что для ревью вручную указывается URL репозитория и вручную же закачивается diff (в отличие от реальной интеграции у CodeCollaborator, когда ревью создаётся двумя кликами прямо из клиента VCS), и заканчивая тем, что комментарии и обсуждения организованы в «старом добром» стиле форума (в отличие от отдельного реал-тайм мини-чата для каждого из замечаний у CodeCollaborator: smartbear.com/images/products/codecollaborator/ccollab-feature-sidebyside.png).
Года 3 назад использовал ещё какую-то (сейчас даже уже не помню название), но та по-моему была тесно привязана к ClearCase (те, кому эти буквы незнакомы — ребята, я вам реально завидую! :)) и была глючной до ужаса. И у неё не было веб-клиента.
Пока что лидирует продукт crucible от Atlassian. Ибо мы юзаем jira, и туда этот продукт хорошо встраивается. Опять же если вдруг попадется достаточно хорошая альтернатива опенсорсная то почему бы и нет. Не всегда дело в деньгах.
Я прошу прощения за некропостинг, но может кто-то ещё будет читать этот топик и ему пригодится.
Пришлось тут покопаться с Review Board, выяснилось, что вручную заливать diff совершенно не обязательно, к нему прилагается комманд-лайн тулза post-review, которая автоматизирует создание и обновление запросов на ревью. Можно дёргать из post-commit hook'а, если нужно. URL репозитария сетапится либо в свойства репозитария (в SVN properties, например) или же один раз прописывается в .reviewboardrc, который кладётся в корень рабочей копии.
Ну то есть всё конечно не так радужно, как у CodeCollaborator, но и не так печально, как вы описали.
У нас — ревью всего нового кода. Но такого варианта нет :)
Насчет пересмотра старого… нацеленно вроде не пересматриваем, разве что заодно с какими-то фиксами.
Проводим ревью нового кода перед коммитом. Никакими дополнительными инструментами не пользуемся — садимся рядом и всё просматриваем-изучаем (благо команда не распределённая). От общения во время ревью мы получаем какую-то пользу как от парного программирования, надеюсь, что хотя бы процентов 5-10% :) (парное программирование, к сожалению, пока не хватило сил/смелости внедрить).
Чтобы не спорить «кто сейчас будет делать ревью?» — на итерацию назначается дежурный. У текущего дежурного ревью делает дежурный по предыдущей итерации.
Мы используем парное программирование — это по сути непрерывное code review прямо во время написания кода.
Но несмотря на это, делаем иногда и code review, и оно даже иногда что-то находит. Но это уже, как правило, просто предложения вроде «а вот можно было бы немного попроще сделать» или «можно было бы попонятнее переменную назвать», а не такие как «ооо, что за код — полная жопа!», которые я наблюдал в других компаниях, где ПП не использовалось.
Проводит ли ваша команда ревизии кода?