Pull to refresh

Comments 41

Существует мнение в «народе», что на территории exUSSR очень плохо обстоят дела с инженерными практиками разработки, в частности с code-review. А так ли это?
Совершенно согласен, никак. Но, например, многие из моих знакомых-разработчиков не проводят ревизий кода, постоянно ссылаясь на давление по срокам, постоянный вброс фич и так далее. Хотя где логика — не ясно. Ведь выдача кода «на гора» без code-review только затянет проект из-за потенциальных багов в нем.
> Ведь выдача кода «на гора» без code-review только затянет проект из-за потенциальных багов в нем.

CR не панацея, и не спасает от всего. Речь в голосовании идет, как я понял, о пересмотре-рефакторинге всего кода, а не только нового.
Да, пересмотр всего кода тоже имелся в виду.

То что CR не панацея, верно. Но грустно смотреть на некоторые компании, которые пытаются давить на тестирование, чтобы те «лучше тестировали», но не хотят приходить к таким вещам как code-review или unit-тесты. Хотя мы уже уходим от темы…
Вы сомневаетесь что автор знает о чем задал вопрос? :)
Я в этом не сомневаюсь :) Я сомневался в том, что правильно понял о чем автор задал вопрос.
Пересмотр кода ведь бывает разный. Например:

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

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

— Архитектор держит в голове всю структуру проекта и следит за каждым коммитом, пинная разработчика если тот коммитает что-то эдакое. Тоже code-review в каком-то смысле :)
У нас, кстати, первый вариант. Для коммита нужно два «аппрува» от коллег. Поэтому нажал «воздержаться».
Наверное, это можно отнести к «или чаще» :)

А если не секрет, используется какие-либо средства для этого? Или всегда на словах?
Используем Review Board.
Это не совсем полноценное ревью, таким образом отслеживаются ошибки связанные, в большей степени, с невнимательностью, чем глобально-архитектурные.
Качественный код редко является целью разработки.
In the wild обычно нужно быстро разработать необходимое приложение.
Если у приложения хоть сколь-нибудь длительный срок поддержки (например, полгода и больше), то без качественного кода потом ой как сложно приходится.

Понятно, что не все заказчики (внешние или внутренние) готовы платить за качество кода, но это больше от непонимания. Если заказчик выбирает быстро и дешево, нужно несколько раз убедиться, что он понимает свой выбор, перед тем как соглашаться на проект. Или не соглашаться :)
Не всё так страшно.
Мне пару раз в год приходится ковыряться в старых (лет восемь) приложениях на PHP — новую функциональность добавить или внезапно проявившийся баг исправить.
Там ужас-ужас-ужас в стиле php3, обращения к бд чередуются выводом html, get-параметры используются как переменные и т.п. Но приложения все боевые, используются многими людьми каждый день, и вмешательства требуют крайне редко. Причём это не какой-нибудь сайт, а «корпоративные» приложения.
Так что качество кода — не главное.
А код свой или чужой «ковыряете»?

Время от времени тоже приходится заниматься своим древним проектом на C++ (99-й год вроде). Там тоже все через одно место написано, но со своим кодом работать, конечно, легче.
>Качественный код редко является целью разработки.

Качественный код является средством при достижении цели «проект, сданный вовремя и не имеющий критических багов».
Проводим, но редко, когда возникают проблемы, и не всего кода.
Надо бы регулярно по идее, штука очень полезная. Все осложняется roadmap'ом и огромным количеством кода и подпроектов.
Свой вариант — ревьюим каждый или почти каждый коммит

Есть ещё такая шутка: "- Что такое кодревью? — Ну так: сидишь, читаешь код, ревёшь"
Хм. А где тут парное программирование. У насс как раз оно, по этому ревью идет на моменте написания кода.
Без code review нельзя сделать очень критичный проект 24x7, кде каждая минута простоя это более чем 1 000 000 евро. Если код не знаем более чем один человек — это в большенстве случаев выброшенный код. Офигенно рулит техник pair programing для подобных проектов.
а у нас времени нет :(
наверное оно «появиться», когда код будем поддерживать…
В конце итерации — поздновато. Чем раньше фидбек, тем лучше. У нас вообще верификация не начинается, пока не будет проведен код-ревью.
Юрий, тут, скорее всего, многое зависит от зрелости команды. Ни в коем случае не камень в ваш огород, но у нас верификация и валидация — это непрерывный каждодневный процесс, «вшитый» в нашу методику разработки. Поэтому к окончанию итерации мы уверены, что мы делаем правильные вещи и делаем эти вещи правильно, а вот как мы их делаем на уровне кода — смотрим, оцениваем, да и просто вся команда проекта знакомится с кодом — вдруг придётся с ним работать.
Тогда получается, что мы с вами используем ревью совсем для разных целей. У нас это
— непрерывный контроль качества кода в процессе реализации,
— дополнение к не всегда хорошо поставленной верификации,
— помощь разработчикам в освоении новых для них частей системы.

У вас же это скорее некая метрика. Тоже интересный подход :)
Перед коммитом проводимь ревью. После комита — верификация.
У нас каждый pull request проходит code review одним или двумя разработчиками (зависит от сложности), без этого код не проходит в основную ветку разработки. То есть обычно разработчик за день читает один-два чужих pull-request'а.
Проводим ревью в конце итерации, это получается раза два в месяц. Выявляем слабые и узкие места. Стараемся исправить в след итерации.
Проводим раз в неделю.

Есть желание пойти дальше. Что посоветуете из огранизационных/инженерных практик? Пока на ум приходит только Design Review для отдельных модулей.
спасибо. А еще знаете какие-нибудь инструменты? Подыскиваем себе
Из бесплатных неплох 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, и туда этот продукт хорошо встраивается. Опять же если вдруг попадется достаточно хорошая альтернатива опенсорсная то почему бы и нет. Не всегда дело в деньгах.

Еще хотелось бы увидеть отзывы по
codereviewtool.com
inspectify.com
groogle.sourceforge.net/

А может даже есть тот кто уже много чего перепробовал…
Я прошу прощения за некропостинг, но может кто-то ещё будет читать этот топик и ему пригодится.

Пришлось тут покопаться с Review Board, выяснилось, что вручную заливать diff совершенно не обязательно, к нему прилагается комманд-лайн тулза post-review, которая автоматизирует создание и обновление запросов на ревью. Можно дёргать из post-commit hook'а, если нужно. URL репозитария сетапится либо в свойства репозитария (в SVN properties, например) или же один раз прописывается в .reviewboardrc, который кладётся в корень рабочей копии.

Ну то есть всё конечно не так радужно, как у CodeCollaborator, но и не так печально, как вы описали.
У нас — ревью всего нового кода. Но такого варианта нет :)
Насчет пересмотра старого… нацеленно вроде не пересматриваем, разве что заодно с какими-то фиксами.
Присоединяюсь. У нас ревью — обязательный процесс перед каждым комитом. Причем должны посмотреть не менее двух человек, помимо автора.
Проводим ревью нового кода перед коммитом. Никакими дополнительными инструментами не пользуемся — садимся рядом и всё просматриваем-изучаем (благо команда не распределённая). От общения во время ревью мы получаем какую-то пользу как от парного программирования, надеюсь, что хотя бы процентов 5-10% :) (парное программирование, к сожалению, пока не хватило сил/смелости внедрить).
Чтобы не спорить «кто сейчас будет делать ревью?» — на итерацию назначается дежурный. У текущего дежурного ревью делает дежурный по предыдущей итерации.
Мы используем парное программирование — это по сути непрерывное code review прямо во время написания кода.

Но несмотря на это, делаем иногда и code review, и оно даже иногда что-то находит. Но это уже, как правило, просто предложения вроде «а вот можно было бы немного попроще сделать» или «можно было бы попонятнее переменную назвать», а не такие как «ооо, что за код — полная жопа!», которые я наблюдал в других компаниях, где ПП не использовалось.
Sign up to leave a comment.

Articles