Как лучше организовать code review?
Если именно нужно code review, чтобы можно было видеть все изменения коммита, а также все комментарии для каждого изменения, чтобы не проревьювленные коммиты не попадали в мастер, можно использовать git + gerrit
Как организовывать ветки/коммиты в SVN/GIT?Вроде все просто, одна атомарная задача - одна ветка.
Вроде все просто, одна атомарная задача - одна веткаЭто когда понятно, что можно от транка отпочковаться.
А если то, что тебе в этой задаче нуна, еще в транк не отлито?
Вроде все просто, одна атомарная задача - одна ветка.И нахуя ветки плодить, как взбесившийся принтер? Чтобы потом в этом говнище сложнее разобраться было?
Это как раз нормально.
Сливаешь в стейбл - трешь ветку
Сливаешь в стейбл - трешь ветку
Как лучше организовать code review?
Review Board:
http://www.reviewboard.org/
Интегрируется с SVN и GIT.
Ты что-то там пишешь, а потом вместо комита в консоли пишешь команду на новое реыью, оно заливает дифф на сервак и делает новое ревью. Можно дифф обновлять.
Короче, опыт мега положителный, если хочешь внедрить — расскажу. Фишка этой борды в том, что она удобна для самого ревью и не надо отдельно делать дифф файл и его куда-то закачивать, ты просто из консоли командой простой заливаешь на ревью.
Ну еще типовая ошибка, когда заводят какую-то программу для проведения code review, в тяжелых случаях с возможностью просматривать diff через веб-интерфейс с возможностью построчного комментирования (как в гитхабе)
А если то, что тебе в этой задаче нуна, еще в транк не отлито?Вмердживаешь в свою ветку другие ветки. Потом можно будет просмотреть только коммиты, сделанные именно в рамках твоей ветки. Например, есть ветка devel, от нее отпочковали featureA и feautureB. Внезапно оказывается, что featureB зависит от featureA. Ты вмердживаешь featureA в featureB. Затем, когда тебе надо посмотреть коммиты или diff который относится именно к featureB, ты пишешь
git log featureB --not featureA --not devel
или
git diff featureB --not featureA --not devel
Главное, при мердже все засквошивать, чтобы история главной ветки была линейна и содержала только атомарные коммиты, одна задача - один коммит.
К сожалению, идиотские веб-интерфейсы типа гитхаба поощряют создание помоечной истории.
А как правильно делать ревью, по твоему?
А как правильно делать ревью, по твоему?Скорее всего, вообще не надо делать, есть способы потратить ресурсы более продуктивно.
Думаю, зачастую внедрение code review происходит так.
Устанавливают систему для код ревью. Кто-то реализует фичу, отправляет ее на ревью. Код попадает к ревьюверу. У него своих задач по горло, глубоко вникать в суть задачи лень и нет времени, к тому же, раз код попал на ревью, фича реализована и работает, что-то переделывать уже смысла нет.
Ревьюверу надо изобразить какую-то активность, не зря же всю движуху затевали. Он быстро пробегает код глазами, находит какой-то маловажный недочет, и оставляет коммент под строками с этим недочетом. Вроде, и код проревьюировал, и время не потратил. Хотя если вдуматься поглубже, в коде могут быть серьезнейшие проблемы, которые при поверхностном просмотре невозможно выявить. Таким образом, код ревью вырождается в бесполезный ритуал, но зато можно гордиться что внедрены такие полезные практики программирования.
Если не использовать системы код ревью с веб-интерфейсом, то как минимум ревьювер должен будет зачекаутить код на свою машину, это уже хорошо. Затем, если нет системы построчного комментирования, то доебаться до мелочи становится невозможно, приходится находить более крупные недочеты.
В идеале, ревьювер вообще не должен смотреть diff, а должен смотреть итоговый код. Просмотр диффа не выявляет копипасту, например.
http://www.atlassian.com/software/crucible/overviewТормозное глючное говнище. Я два раза в двух разных конторах был свидетелем попыток внедрения этого поделия. В обоих случаях — фэйл, причем именно из-за тормозов. С приемлемой скоростью оно работает только для проектов размеров нано- и пико-.
Маркетинговая политика Атлассиана такая. Есть два основных продукта: Jira и Confluence, которые они пилят и вылизывают до совершенства. А всё остальное — довесочный треш. Сначала ты покупаешь связку Jira/Confluence, радуешься их качеству, а позже, когда приходит желание внедрить code review, то по инерции докупаешь Crucible/Fisheye. Тут-то тебя и ждет фэйл.
В идеале, ревьювер вообще не должен смотреть diff, а должен смотреть итоговый код. Просмотр диффа не выявляет копипасту, например.При добавлении каждой фичи надо просматривать весь код?
Сканируя каждый раз весь код, копипасту пропустишь с бОльшей вероятностью.
Насчёт модели ветвления git - мы используем такуюЗа ссылку спасибо.
http://m.habrahabr.ru/post/106912/
Я в восторге
Но там нет про code review.
Там ничего не сказано про подмердживание в функциональные ветки из trunk. Когда функциональная ветка существует долго, она может сильно разойтись с trunk, другими словами, чем старше функциональная ветка, тем выше вероятность конфликтов при мердже. Поэтому надо переодически мерджить trunk в функциональную ветку.
Хочется следующего.
На code review попадают небольшие аккуратные коммиты. Небольшие — это что-то типа ежедневных или не больше 200-400 строк кода.
Каждый коммит, попавший на ревью, может быть либо отклонен с просьбой исправить, либо принят.
Это всё должно отражаться в истории в SVN.
Запланирован переход на GIT.
На code review попадают небольшие аккуратные коммиты. Небольшие — это что-то типа ежедневных или не больше 200-400 строк кода.
Каждый коммит, попавший на ревью, может быть либо отклонен с просьбой исправить, либо принят.
Это всё должно отражаться в истории в SVN.
Запланирован переход на GIT.
Получается такая схема веток:
TargetBranch
|__CodeReview
|__BeforeCodeReview
Сначала коммиты делаются в ветку BeforeCodeReview. После завершения небольшой законченной задачи выбранное множество коммитов мерджится одним коммитом в ветку CodeReview с ясным описанием задачи. Если по этой задаче замечаний нет, то коммит с описанием переносится в ветку TargetBranch. Если замечания есть, то в ветку CodeReview делается исправляющий коммит.
TargetBranch
|__CodeReview
|__BeforeCodeReview
Сначала коммиты делаются в ветку BeforeCodeReview. После завершения небольшой законченной задачи выбранное множество коммитов мерджится одним коммитом в ветку CodeReview с ясным описанием задачи. Если по этой задаче замечаний нет, то коммит с описанием переносится в ветку TargetBranch. Если замечания есть, то в ветку CodeReview делается исправляющий коммит.
Непонятно, почему коммиты для ревью должны быть небольшими. Ревьюировать удобнее наоборот большими кусками. Например, кто-то одним коммитом написал класс, а другим коммитом вставил использование класса. В момент первого коммита неясно, правильное ли API у класса.
Непонятно, почему описание задачи появляется в момент code review.
Идею делать две ветки - code review и before code review я не понял, идея нестандартная. Как я понял, там перемешиваются коммиты от разных задач, потом их надо как-то руками переносить. Это мне кажется неудобным.
Непонятно, почему описание задачи появляется в момент code review.
Идею делать две ветки - code review и before code review я не понял, идея нестандартная. Как я понял, там перемешиваются коммиты от разных задач, потом их надо как-то руками переносить. Это мне кажется неудобным.
При добавлении каждой фичи надо просматривать весь код?Надо находить код, который реализует фичу, и смотреть на него в контексте окружающего кода.
Но это в идеальном мире, в реальном мире думаю полезнее тратить время на предварительное продумывание, например.
Думаю, код ревью используется в основном где есть избыток программистов, их надо чем-то занять.
Я короче нифига не знаю в чём понт код ревью у вас, но у нас это как бы возможность а) донести до говнокодеров идею что они кодят говно, и мы это говно не пропустим, пишите С++ правильно б) внезапно обнаруживается что они юзают наши АПИ совсем не так как мы предполагали, тогда либо б1) иди прочитай гайдлайны в документацее, для кого мы их пишем блджад, либо б2) оппа, надо делать более правильное АПИ.
Главное в этом всём — что ревью делается сениор программистами которые знают Как Надо, и имеют власть указать чуваку или чувихе что нет, Так не Надо, поправь или GTFO. Ну и типа не для того чтобы найти очевидные ошибки: если хочется этого — ну ок, скажите своим джуниорам ревьюить код других джуниоров, я не вижу в чём тут может появиться проблема, "у тебя тут баг кстате", "ок поправил".
Как бы нет проблем, как бы проблемы возникают только когда кто-то в результате ревью говорит что очевидных багов нет, но код надо писать по-другому. Ну вот поэтому такого типа ревью должны делаться старшими программистами и если я говорю писать по-другому, то чувак идёт и переписывает как я сказал.
(А ревью между старшими программистами лучше не делать официально, мы и так это делаем сами обычно. Ну, у нас мы делаем)
EDIT: то есть я к чему, есть два типа ревью, 1) найти очевидные баги, 2) проэнфорсить Правильный Стиль Кода. Ну и вот исходя из моего опыта оба типа ревью не требуют технических костылей на самом деле, и они только мешают (мы юзали Jira фигню, например — мешает!). Про очевидные баги ты просто говоришь и программистка их фиксит, нечего обсуждать, про некрасивый код ты авторитетно говоришь с позиции авторитета и программист фиксит свой код. Всё это делается проще всего через чятик, типа lync или gtalk или что вы используете, фиксировать это как-то конкретно не нужно, потому что в обоих случаях нет необходимости ревьюить ревью, если это баг то он был пофикшен, если это был некрасивый код то он был переписан красиво.
Типа, технические костыли могут быть нужны только если возникнет какой-то конфликт который не может разрешиться через "Я сказал делать так"/"тут очевидный баг". То есть твои джуниоры почему-то ревьюят стиль (и ты не можешь сразу призвать старшого), или ты попытался стравить сеньоров друг с другом зачем-то.
Ну и вот. Ключ к хорошим код-ревью не в технических костылях, а в установлении правильной иерархии (и отделении bug-finding reviews от code style).
Главное в этом всём — что ревью делается сениор программистами которые знают Как Надо, и имеют власть указать чуваку или чувихе что нет, Так не Надо, поправь или GTFO. Ну и типа не для того чтобы найти очевидные ошибки: если хочется этого — ну ок, скажите своим джуниорам ревьюить код других джуниоров, я не вижу в чём тут может появиться проблема, "у тебя тут баг кстате", "ок поправил".
Как бы нет проблем, как бы проблемы возникают только когда кто-то в результате ревью говорит что очевидных багов нет, но код надо писать по-другому. Ну вот поэтому такого типа ревью должны делаться старшими программистами и если я говорю писать по-другому, то чувак идёт и переписывает как я сказал.
(А ревью между старшими программистами лучше не делать официально, мы и так это делаем сами обычно. Ну, у нас мы делаем)
EDIT: то есть я к чему, есть два типа ревью, 1) найти очевидные баги, 2) проэнфорсить Правильный Стиль Кода. Ну и вот исходя из моего опыта оба типа ревью не требуют технических костылей на самом деле, и они только мешают (мы юзали Jira фигню, например — мешает!). Про очевидные баги ты просто говоришь и программистка их фиксит, нечего обсуждать, про некрасивый код ты авторитетно говоришь с позиции авторитета и программист фиксит свой код. Всё это делается проще всего через чятик, типа lync или gtalk или что вы используете, фиксировать это как-то конкретно не нужно, потому что в обоих случаях нет необходимости ревьюить ревью, если это баг то он был пофикшен, если это был некрасивый код то он был переписан красиво.
Типа, технические костыли могут быть нужны только если возникнет какой-то конфликт который не может разрешиться через "Я сказал делать так"/"тут очевидный баг". То есть твои джуниоры почему-то ревьюят стиль (и ты не можешь сразу призвать старшого), или ты попытался стравить сеньоров друг с другом зачем-то.
Ну и вот. Ключ к хорошим код-ревью не в технических костылях, а в установлении правильной иерархии (и отделении bug-finding reviews от code style).
о через чятик,Через чятик конечно хорошо, но не удобно туда куски кода копипастить, исправителю надо еще найти этот код у себя.
Кстати, а исправления по код ревью, оформляются в виде отдельных коммитов или амендятся к изначальном? Если амендятся, то повторное ревью делается?
Непонятно, почему коммиты для ревью должны быть небольшими. Ревьюировать удобнее наоборот большими кусками. Например, кто-то одним коммитом написал класс, а другим коммитом вставил использование класса. В момент первого коммита неясно, правильное ли API у класса.Если класс без вызовов попадет в ветку CodeReview, то этот коммит будет отклонен ревьюером с пояснением "ненужный код".
Непонятно, почему описание задачи появляется в момент code review.
Идею делать две ветки - code review и before code review я не понял, идея нестандартная. Как я понял, там перемешиваются коммиты от разных задач, потом их надо как-то руками переносить. Это мне кажется неудобным.
А какая стандартная система веток для code review?
Не обязательно перемешиваются. TargetBranch может быть функциональной веткой. Поинт в том, что где-то должны образовываться коммиты для review. Они должны быть перед коммитами в TargetBranch, отсюда получаем, что надо делать ветку CodeReview. А для того, чтобы разработчик мог делать свои частые коммиты, а потом объединить их в один для ревью, для этого делается BeforeCodeReview.
Там ничего не сказано про подмердживание в функциональные ветки из trunk. Когда функциональная ветка существует долго, она может сильно разойтись с trunk, другими словами, чем старше функциональная ветка, тем выше вероятность конфликтов при мердже. Поэтому надо переодически мерджить trunk в функциональную ветку.согласен, у нас тоже есть некоторые отличия от этой статьи
1. Периодически подливается в персональную ветку develop, release или stable в зависимости от того, откуда отбранчевалась первоначальная задача
2. ветка stable после каждого коммита подливается в release, в свою очередь release подливается в develop. В обратную сторону подливать запрещено.
3. Персональные ветки не удаляются, все ветки заливаются в удаленный репозиторий. Все программисты видят все коммиты других программистов.
Если нужно будет сделать патч для старой версии, можно будет забрать изменения из персональной ветки с помощью cherry pick
4. Репозиторий синхронизирован с jira, crucible, билд-серверами. Билд-серверы делают сборки по каждому коммиту в develop, release, master
А какая стандартная система веток для code review?Мы не делаем отдельных веток для ревью кода. У каждого разработчика есть свой гит-репозитарий форкнутый от мастера, разработчик имеет право пушить только в него. Также он подключает мастер-репозитарий как удаленный и периодически ребэйзиться на него(таким образом недоделанные и непроревьювленные фичи сильно не протухают). Когда сделана некая работа, разработчик говорит заберите мои коммиты в мастер, кто-то это смотрит, если ок, то переносит в мастер, если не ок, то исполнитель переделывает и цикл повторяется. Когда команда сидит в одной комнате, в полне ок, иначе становится не удобно. Для автоматизации процесса можно использовать gerrit.
ветка stable после каждого коммита подливается в releaseПочему после каждого? Просто несколько раз в неделю
Почему после каждого? Просто несколько раз в неделюТот, кто будет подливать раз в неделю, будет разрешать конфликты за себя и за того парня?
Карма такая
На code review попадают небольшие аккуратные коммиты. Небольшие — это что-то типа ежедневных или не больше 200-400 строк кода.о, я так делал через bitbucket (приватный реп)
Каждый коммит, попавший на ревью, может быть либо отклонен с просьбой исправить, либо принят.
коммитеры делают тебе пул-реквесты, их можно отклонять с комментарием или комментить любую строчку
без особого жонглирования ветками, время экономит сильно
не в курсе правда, можно ли это локально развернуть
У нас была практика делать ревью до коммита. Подсаживаешься к соседу, и смотрите вместе diff. Можно было и удаленно через Skype. Таким образом убирались глупые ошибки, плюс, оба были в курсе работы другого, на случай, если придется заменить.
Коммиты делаются в одну ветку default. Релиз делается веткой, в которой только исправляются ошибки.
Для примера, репозиторий nginx. На мой взгляд, это эталон. А толстые git репозитории с кучей веток для каждой фичи - это лишнее.
И еще, запуск тестов после каждого коммита.
Коммиты делаются в одну ветку default. Релиз делается веткой, в которой только исправляются ошибки.
Для примера, репозиторий nginx. На мой взгляд, это эталон. А толстые git репозитории с кучей веток для каждой фичи - это лишнее.
И еще, запуск тестов после каждого коммита.
Если уж перешли к обсуждению веток, релизов и патчей, то мне импонирует модель разработки базовой системы FreeBSD: http://www.freebsd.org/doc/en/books/dev-model/model-summary... (картинка не очень, но другую за пару минут не нагуглил).
Параллельная разработка одновременно в двух или нескольких ветках может вызвать накопление технического долга, который в конечном итоге будет необходимо восполнить для слияния изменений воедино. Чем больше изменений, которые сделаны изолировано, тем больше итоговый долг.
Странно обсуждать code review не упомянув Gerrit. И по-моему он делает ровно то, что тебе нужно.
У него есть кучка багов и существует несколько более дружелюбных аналогов типа упомянутого reviewboard, но Gerrit сейчас - мэйнстрим, легко прикручивается ко всяким сервисам типа того же Дженкинса.
У него есть кучка багов и существует несколько более дружелюбных аналогов типа упомянутого reviewboard, но Gerrit сейчас - мэйнстрим, легко прикручивается ко всяким сервисам типа того же Дженкинса.
Странно обсуждать code review не упомянув Gerrit. И по-моему он делает ровно то, что тебе нужно.Он же древний и убогий. Мощный, конечно, но морально устарел.
Для Code Review я бы посоветовал Upsource. Быстро, удобно, современно.
qwert
Оставить комментарий
6yrop
Какой временной режим выбрать?Как организовывать ветки/коммиты в SVN/GIT?