Комментарии 41
Существует мнение в «народе», что на территории exUSSR очень плохо обстоят дела с инженерными практиками разработки, в частности с code-review. А так ли это?
А как без этого разрабатывать качественный код?
Совершенно согласен, никак. Но, например, многие из моих знакомых-разработчиков не проводят ревизий кода, постоянно ссылаясь на давление по срокам, постоянный вброс фич и так далее. Хотя где логика — не ясно. Ведь выдача кода «на гора» без code-review только затянет проект из-за потенциальных багов в нем.
> Ведь выдача кода «на гора» без code-review только затянет проект из-за потенциальных багов в нем.
CR не панацея, и не спасает от всего. Речь в голосовании идет, как я понял, о пересмотре-рефакторинге всего кода, а не только нового.
CR не панацея, и не спасает от всего. Речь в голосовании идет, как я понял, о пересмотре-рефакторинге всего кода, а не только нового.
Да, пересмотр всего кода тоже имелся в виду.
То что CR не панацея, верно. Но грустно смотреть на некоторые компании, которые пытаются давить на тестирование, чтобы те «лучше тестировали», но не хотят приходить к таким вещам как code-review или unit-тесты. Хотя мы уже уходим от темы…
То что CR не панацея, верно. Но грустно смотреть на некоторые компании, которые пытаются давить на тестирование, чтобы те «лучше тестировали», но не хотят приходить к таким вещам как code-review или unit-тесты. Хотя мы уже уходим от темы…
Вы сомневаетесь что автор знает о чем задал вопрос? :)
Я в этом не сомневаюсь :) Я сомневался в том, что правильно понял о чем автор задал вопрос.
Пересмотр кода ведь бывает разный. Например:
— Перекрестное ревью коммитов (т.е. один разработчик проверяет код, закоммитанный другим)
— Все разработчики собираются, допустим, раз в месяц, выделяют несколько дней и досконально изучают весь код, обсуждая (и исправляя) спорные места.
— Архитектор держит в голове всю структуру проекта и следит за каждым коммитом, пинная разработчика если тот коммитает что-то эдакое. Тоже code-review в каком-то смысле :)
Пересмотр кода ведь бывает разный. Например:
— Перекрестное ревью коммитов (т.е. один разработчик проверяет код, закоммитанный другим)
— Все разработчики собираются, допустим, раз в месяц, выделяют несколько дней и досконально изучают весь код, обсуждая (и исправляя) спорные места.
— Архитектор держит в голове всю структуру проекта и следит за каждым коммитом, пинная разработчика если тот коммитает что-то эдакое. Тоже code-review в каком-то смысле :)
Качественный код редко является целью разработки.
In the wild обычно нужно быстро разработать необходимое приложение.
In the wild обычно нужно быстро разработать необходимое приложение.
Если у приложения хоть сколь-нибудь длительный срок поддержки (например, полгода и больше), то без качественного кода потом ой как сложно приходится.
Понятно, что не все заказчики (внешние или внутренние) готовы платить за качество кода, но это больше от непонимания. Если заказчик выбирает быстро и дешево, нужно несколько раз убедиться, что он понимает свой выбор, перед тем как соглашаться на проект. Или не соглашаться :)
Понятно, что не все заказчики (внешние или внутренние) готовы платить за качество кода, но это больше от непонимания. Если заказчик выбирает быстро и дешево, нужно несколько раз убедиться, что он понимает свой выбор, перед тем как соглашаться на проект. Или не соглашаться :)
Не всё так страшно.
Мне пару раз в год приходится ковыряться в старых (лет восемь) приложениях на PHP — новую функциональность добавить или внезапно проявившийся баг исправить.
Там ужас-ужас-ужас в стиле php3, обращения к бд чередуются выводом html, get-параметры используются как переменные и т.п. Но приложения все боевые, используются многими людьми каждый день, и вмешательства требуют крайне редко. Причём это не какой-нибудь сайт, а «корпоративные» приложения.
Так что качество кода — не главное.
Мне пару раз в год приходится ковыряться в старых (лет восемь) приложениях на PHP — новую функциональность добавить или внезапно проявившийся баг исправить.
Там ужас-ужас-ужас в стиле php3, обращения к бд чередуются выводом html, get-параметры используются как переменные и т.п. Но приложения все боевые, используются многими людьми каждый день, и вмешательства требуют крайне редко. Причём это не какой-нибудь сайт, а «корпоративные» приложения.
Так что качество кода — не главное.
>Качественный код редко является целью разработки.
Качественный код является средством при достижении цели «проект, сданный вовремя и не имеющий критических багов».
Качественный код является средством при достижении цели «проект, сданный вовремя и не имеющий критических багов».
Проводим, но редко, когда возникают проблемы, и не всего кода.
Надо бы регулярно по идее, штука очень полезная. Все осложняется roadmap'ом и огромным количеством кода и подпроектов.
Надо бы регулярно по идее, штука очень полезная. Все осложняется roadmap'ом и огромным количеством кода и подпроектов.
Свой вариант — ревьюим каждый или почти каждый коммит
Есть ещё такая шутка: "- Что такое кодревью? — Ну так: сидишь, читаешь код, ревёшь"
Есть ещё такая шутка: "- Что такое кодревью? — Ну так: сидишь, читаешь код, ревёшь"
Хм. А где тут парное программирование. У насс как раз оно, по этому ревью идет на моменте написания кода.
Без code review нельзя сделать очень критичный проект 24x7, кде каждая минута простоя это более чем 1 000 000 евро. Если код не знаем более чем один человек — это в большенстве случаев выброшенный код. Офигенно рулит техник pair programing для подобных проектов.
а у нас времени нет :(
наверное оно «появиться», когда код будем поддерживать…
наверное оно «появиться», когда код будем поддерживать…
Да, в конце каждой итерации
Юрий, тут, скорее всего, многое зависит от зрелости команды. Ни в коем случае не камень в ваш огород, но у нас верификация и валидация — это непрерывный каждодневный процесс, «вшитый» в нашу методику разработки. Поэтому к окончанию итерации мы уверены, что мы делаем правильные вещи и делаем эти вещи правильно, а вот как мы их делаем на уровне кода — смотрим, оцениваем, да и просто вся команда проекта знакомится с кодом — вдруг придётся с ним работать.
Тогда получается, что мы с вами используем ревью совсем для разных целей. У нас это
— непрерывный контроль качества кода в процессе реализации,
— дополнение к не всегда хорошо поставленной верификации,
— помощь разработчикам в освоении новых для них частей системы.
У вас же это скорее некая метрика. Тоже интересный подход :)
— непрерывный контроль качества кода в процессе реализации,
— дополнение к не всегда хорошо поставленной верификации,
— помощь разработчикам в освоении новых для них частей системы.
У вас же это скорее некая метрика. Тоже интересный подход :)
Перед коммитом проводимь ревью. После комита — верификация.
У нас каждый pull request проходит code review одним или двумя разработчиками (зависит от сложности), без этого код не проходит в основную ветку разработки. То есть обычно разработчик за день читает один-два чужих pull-request'а.
Проводим ревью в конце итерации, это получается раза два в месяц. Выявляем слабые и узкие места. Стараемся исправить в след итерации.
Проводим раз в неделю.
Есть желание пойти дальше. Что посоветуете из огранизационных/инженерных практик? Пока на ум приходит только Design Review для отдельных модулей.
Есть желание пойти дальше. Что посоветуете из огранизационных/инженерных практик? Пока на ум приходит только Design Review для отдельных модулей.
Перед каждым коммитом проводится ревью (минимум 2 человека) при помощи CodeCollaborator.
спасибо. А еще знаете какие-нибудь инструменты? Подыскиваем себе
Из бесплатных неплох 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 (те, кому эти буквы незнакомы — ребята, я вам реально завидую! :)) и была глючной до ужаса. И у неё не было веб-клиента.
Начиная с того, что «интеграция» с VCS заключается в том, что для ревью вручную указывается URL репозитория и вручную же закачивается diff (в отличие от реальной интеграции у CodeCollaborator, когда ревью создаётся двумя кликами прямо из клиента VCS), и заканчивая тем, что комментарии и обсуждения организованы в «старом добром» стиле форума (в отличие от отдельного реал-тайм мини-чата для каждого из замечаний у CodeCollaborator: smartbear.com/images/products/codecollaborator/ccollab-feature-sidebyside.png).
Года 3 назад использовал ещё какую-то (сейчас даже уже не помню название), но та по-моему была тесно привязана к ClearCase (те, кому эти буквы незнакомы — ребята, я вам реально завидую! :)) и была глючной до ужаса. И у неё не было веб-клиента.
Пока что лидирует продукт crucible от Atlassian. Ибо мы юзаем jira, и туда этот продукт хорошо встраивается. Опять же если вдруг попадется достаточно хорошая альтернатива опенсорсная то почему бы и нет. Не всегда дело в деньгах.
Еще хотелось бы увидеть отзывы по
codereviewtool.com
inspectify.com
groogle.sourceforge.net/
А может даже есть тот кто уже много чего перепробовал…
Еще хотелось бы увидеть отзывы по
codereviewtool.com
inspectify.com
groogle.sourceforge.net/
А может даже есть тот кто уже много чего перепробовал…
Я прошу прощения за некропостинг, но может кто-то ещё будет читать этот топик и ему пригодится.
Пришлось тут покопаться с Review Board, выяснилось, что вручную заливать diff совершенно не обязательно, к нему прилагается комманд-лайн тулза post-review, которая автоматизирует создание и обновление запросов на ревью. Можно дёргать из post-commit hook'а, если нужно. URL репозитария сетапится либо в свойства репозитария (в SVN properties, например) или же один раз прописывается в .reviewboardrc, который кладётся в корень рабочей копии.
Ну то есть всё конечно не так радужно, как у CodeCollaborator, но и не так печально, как вы описали.
Пришлось тут покопаться с Review Board, выяснилось, что вручную заливать diff совершенно не обязательно, к нему прилагается комманд-лайн тулза post-review, которая автоматизирует создание и обновление запросов на ревью. Можно дёргать из post-commit hook'а, если нужно. URL репозитария сетапится либо в свойства репозитария (в SVN properties, например) или же один раз прописывается в .reviewboardrc, который кладётся в корень рабочей копии.
Ну то есть всё конечно не так радужно, как у CodeCollaborator, но и не так печально, как вы описали.
У нас — ревью всего нового кода. Но такого варианта нет :)
Насчет пересмотра старого… нацеленно вроде не пересматриваем, разве что заодно с какими-то фиксами.
Насчет пересмотра старого… нацеленно вроде не пересматриваем, разве что заодно с какими-то фиксами.
Проводим ревью нового кода перед коммитом. Никакими дополнительными инструментами не пользуемся — садимся рядом и всё просматриваем-изучаем (благо команда не распределённая). От общения во время ревью мы получаем какую-то пользу как от парного программирования, надеюсь, что хотя бы процентов 5-10% :) (парное программирование, к сожалению, пока не хватило сил/смелости внедрить).
Чтобы не спорить «кто сейчас будет делать ревью?» — на итерацию назначается дежурный. У текущего дежурного ревью делает дежурный по предыдущей итерации.
Чтобы не спорить «кто сейчас будет делать ревью?» — на итерацию назначается дежурный. У текущего дежурного ревью делает дежурный по предыдущей итерации.
Мы используем парное программирование — это по сути непрерывное code review прямо во время написания кода.
Но несмотря на это, делаем иногда и code review, и оно даже иногда что-то находит. Но это уже, как правило, просто предложения вроде «а вот можно было бы немного попроще сделать» или «можно было бы попонятнее переменную назвать», а не такие как «ооо, что за код — полная жопа!», которые я наблюдал в других компаниях, где ПП не использовалось.
Но несмотря на это, делаем иногда и code review, и оно даже иногда что-то находит. Но это уже, как правило, просто предложения вроде «а вот можно было бы немного попроще сделать» или «можно было бы попонятнее переменную назвать», а не такие как «ооо, что за код — полная жопа!», которые я наблюдал в других компаниях, где ПП не использовалось.
Зарегистрируйтесь на Хабре, чтобы оставить комментарий
Проводит ли ваша команда ревизии кода?