Какой лучший способ просмотреть код, прежде чем он будет передан в транк? (СВН)

14

Каков наилучший способ просмотра кода перед его передачей в магистраль SVN? Одна идея, о которой я думаю, - это заставить разработчика зафиксировать свой код в ветке, а затем просмотреть свой код при объединении версий ветки в ствол. Это хорошая практика? Если нет, что еще можно сделать, чтобы просмотреть код, прежде чем он будет передан в транк?

Meysam
источник
2
Вы можете взглянуть на некоторые инструменты, такие как Crucible, которые поддерживают предварительные проверки.
Gyan aka Gary Buyn
3
какая-либо причина не просматривать код после того, как это передано в транк?
Комнат
3
@gnat Ну, я думаю, что лучше, чтобы младшие разработчики зафиксировали свой код где-то еще, а затем слили эти изменения в ствол старшим разработчиком после того, как он / она проверит их и убедится, что эти изменения подходят для ствола. Вы знаете, старший разработчик после просмотра кода, переданного непосредственно в ствол, может решить, что у этого кода есть проблемы, и его следует откатить. Мы могли бы предотвратить этот проблемный код в первую очередь. Вот и вся идея.
Meysam
ты пробовал другой путь или это только догадки?
комар

Ответы:

12

Хотя есть две школы - то, что вы предлагаете или «проверяете, прежде чем совершать». Большинство различий можно рассматривать как негативы и / или позитивы. - Например, нет отслеживания изменений, вызванных проверкой - хотите ли вы рассматривать их как отдельные коммиты, или вас интересует только финальная работа?

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

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

Это во многом зависит от командной культуры и опыта. Какой модели вы доверяете, какова ее основная цель для обзоров? Я лично предпочитаю проверку после коммита, так как она позволяет отслеживать изменения в результате проверки. Теперь мы используем Git и Gerrit, поскольку они обеспечивают хороший баланс между двумя вариантами.

mattnz
источник
1
Постоянное создание ветвей и повторное объединение - это недостаток, который значительно перевешивает потенциально (и безвозвратно) загрязняющий ствол. Обычно главная директива для управления версиями - «не нарушать сборку». Если вы можете сделать это, нет никакой реальной угрозы в регистрации, а все остальное просто корректировка после факта.
Спенсер Кормос
Просмотр после фиксации в ветке хорошо работает с использованием ветвей функций: вы запускаете новую ветку для каждой новой функции или исправления ошибки. Когда он завершен и прошел проверку, он объединяется с транком. Таким образом, ствол содержит только полные, проверенные изменения. Поскольку ветви функций недолговечны, слияния, как правило, тривиальны. Другое преимущество состоит в том, что ствол содержит только полные функции и исправления - все, что наполовину испечено, будет существовать только на ветке.
Стивен С. Сталь
7
  1. Перед фиксацией запустите 'svn diff', чтобы сгенерировать файл патча.
  2. Отправить файл патча рецензенту, который применяет его к чистой копии ствола.
  3. Рецензент просматривает изменения, используя программу просмотра различий.
  4. Рецензент выполняет сборку и запускает тесты.
  5. Если все идет хорошо, сообщите разработчику, что он может проверить свои изменения. Если
    есть проблемы, разработчик возвращается к шагу 1.
Чарльз Э. Грант
источник
5

Там идеальный мир, а потом реальный мир.

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

в реальном мире все по-другому. Бизнес хочет , чтобы изменение жить прямо сейчаси скажет вам с совершенно прямым лицом, что да, у вас будет время, чтобы очистить код и добавить тестовые примеры позже. Вероятно, у вас не будет времени на проверку кода, и процент кода, покрываемого тестами, постоянно уменьшается. Основная причина для проверки кода заключается в том, что начинающие разработчики учатся у старших разработчиков (когда есть время для этого), когда более опытный человек просматривает изменения и предлагает «лучшие способы ведения дел (TM)». У вас будут старшие разработчики, которые просто будут комментировать непросмотренный код. Ветвление только для проверки кода, а затем слияние - огромная трата времени. Один из способов решения этой проблемы - объявить регулярное еженедельное (или около того) двухчасовое совещание группы, на котором вы выбираете одно или два недавних изменения, над которыми работали сотрудники, в сжатые сроки и заставляете своих авторов «представлять» их подход заключается в совместном рассмотрении кода на проекторе или чем-то подобном. Это может привести к некоторым интересным дискуссиям (как правило, совсем не по теме), но в целом улучшает понимание того, как это сделать правильно. Кроме того, необходимость представления вашего кода заставляет некоторых людей делать это лучше ;-)

Или вам повезет, и вы начнете работать в реальной среде, где это не так суетно, программисты действительно ценят то, что они делают, а не злоупотребляют, и есть время сделать все правильно. В этом случае мой ответ будет таким: попробуйте некоторые из различных методов, предложенных в ответах здесь, и посмотрите, какой из них подходит вашей команде и как вы работаете лучше всего.

Амос М. Карпентер
источник
+1 за идею еженедельного обзора. Я мог бы попробовать это
Джейми Тейлор
@JamieTaylor: Ну, это немного компромисс - очевидно, если у вас (и у вашей команды разработчиков) есть время, я бы рекомендовал вместо этого обзоры полного кода. Но это хороший способ поделиться знаниями внутри команды.
Амос М. Карпентер
2

Филиалы должны работать нормально, исходя из моего опыта их использования в предварительных обзорах на предыдущей работе.

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

Поскольку вы, похоже, собираетесь использовать предварительные проверки для всех изменений, вам может потребоваться управлять большим количеством веток. Если вы ожидаете, что разработчик будет вносить одно «проверяемое» изменение в среднем в неделю, у вас будет около 50 веток в год на каждого разработчика в команде. Если вы используете меньшие куски работы - например, занимающие 1, 2, 3 ... дня - умножьте 50 на 2, 3, 5 ... соответственно.

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

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

Устанавливать, отслеживать и разрешать конфликты, связанные со сроками проверки кода. Согласно моим воспоминаниям о предварительных проверках к обычным изменениям, с которыми я сталкивался в одном из прошлых проектов, разумный срок составляет около 3 дней, и время, чтобы начать беспокоиться, это когда проверка не завершена более чем через 1 день после представления.

Для сравнения, в обзорах после фиксации эти требования намного более мягкие (я использую 2-недельный срок и начинаю беспокоиться через 1 неделю) - но поскольку вы нацелены на предварительные обзоры, это, вероятно, неинтересно.

2. конфликты слияния при отправке проверенного кода

Что делать, если коммит для проверенного кода заблокирован конфликтующими изменениями, совершенными кем-то еще, пока код ждал проверки?

Пара вариантов для рассмотрения

  • Откат к началу и требует от разработчиков повторной реализации и пересмотра изменений.
    В этом случае вам может потребоваться устранить негативное влияние на моральный дух команды, которое это может (будет!) иметь.
  • передать ответственность за слияние другому члену команды («мастер слияния»).
    В этом случае вам также необходимо решить, должны ли слияния как таковые проходить предварительную проверку или нет, и если да, то что делать в случае, если это слияние в свою очередь встречает другой конфликт.
  • игнорируйте изменения, внесенные в проверенный код на этапе слияния.
    В этом случае вам может потребоваться устранить негативное влияние на командный дух, связанный с тем, что принятый код отличается от того, который был проверен.
  • Придумайте способ избежать конфликтов.
    Простой подход заключается в том, чтобы позволить только одному разработчику одновременно изменять определенный набор файлов - хотя это не защитит вас от изменений, которые не изменяют файлы напрямую, а влияют на них посредством внутренних изменений API. , Вы также можете обнаружить, что такая «пессимистическая блокировка» делает общесистемные изменения и глубокий рефакторинг довольно проблематичным.

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

3. загрузить разработчика, который ждет обзора

Установите четкую политику в отношении того, должен ли разработчик, представивший рецензию, переключиться на новое задание или заняться чем-то другим (например, преследовать рецензента)

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

комар
источник
0

Любая часть разработки, требующая проверки, должна находиться в отдельной ветке. Таким образом, ветвь должна существовать до того, как придет время для обзора. Тогда шаг должен быть:

  1. Рассмотрение
  2. Пересмотреть (возможно)
  3. вернуться к обзору (возможно)
  4. Слить в багажник

Слияние - трудная часть. Чем дольше ветвь остается независимой, тем сложнее будет слить ее обратно в ствол. (Это также может быть сложнее проверить.)

Перекрестное слияние является возможным решением. Перед объединением в магистраль (шаг 4 или даже раньше, скажем, до шага 3 или шага 1), объедините магистраль в ветвь. Разработчик может сделать это и протестировать. Тогда ветка догоняет ствол и становится легче слить его в ствол. Слияние с багажником лучше всего делать вам или тому, кто отвечает за багажник.

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

Удай Редди
источник