0,0
рейтинг
7 декабря 2015 в 13:20

Разработка → Лучший Pull Request из песочницы

Относительно недавно мне посчастливилось присоединиться к команде разработки Bitbucket Server в Atlassian (до сентября он был известен как Stash). В какой-то момент мне стало любопытно, как этот продукт освещён на Хабре, и к моему удивлению, нашлось лишь несколько заметок о нём, подавляющее большинство которых на сегодняшний день уже устарело.

В связи с этим я решил опробовать себя в роли рассказчика и затронуть техническую сторону Bitbucket. Прошу не рассматривать моё намерение как попытку рекламы, ибо я совершенно не преследую эту цель. Если эта статья обнаружит интерес со стороны читателей, я буду рад развивать тему и постараюсь ответить на возникающие вопросы.

Позвольте начать с перевода статьи Тима Петтерсена «A better pull request» о том, как должен выглядеть pull request, чтобы наиболее эффективно решать возложенную на него задачу.

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

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



Если вы используете Git, то наверняка пользуетесь и pull request-ами. Они в той или иной форме существуют с момента появления распределённых систем управления версиями. До того, как Bitbucket и GitHub предложили удобный веб-интерфейс, pull request мог представлять собой простое письмо от Алисы с просьбой забрать какие-то изменения из её репозитория. Если они были стóящими, вы могли выполнить несколько команд, чтобы влить эти изменения в вашу основную ветку master:

$ git remote add alice git://bitbucket.org/alice/bleak.git
$ git checkout master
$ git pull alice master

Разумеется, включать изменения от Алисы в master не глядя — это далеко не лучшая идея: ведь master содержит код, который вы собираетесь поставлять клиентам, а потому наверняка хотите внимательно следить за тем, что в него попадает. Более правильный путь, чем простое включение изменений в master, — это сначала влить их в отдельную ветку и проанализировать перед тем, как cливать в master:

$ git fetch alice
$ git diff master...alice/master

Приведённая команда git diff с синтаксисом трёх точек (в дальнейшем «triple dot» git diff) покажет изменения между вершиной ветки alice/branch и её merge base — общим предком с локальной веткой master, иначе говоря, с точкой расхождения истории коммитов этих веток. В сущности, это будут ровно те изменения, которые Алиса просит нас включить в основную ветку.


git diff master...alice/master эквивалентен git diff A B

На первый взгляд, это кажется разумным способом проверки изменений pull request-а. Действительно, на момент написания статьи, именно такой алгоритм сравнения применяется в реализации pull request-ов в большинстве инструментов, предоставляющих хостинг git-репозиториев.

Несмотря на это, есть несколько проблем в использовании «triple dot» git diff для анализа изменений pull request-а. В реальном проекте основная ветка, скорее всего, будет сильно отличаться от любой ветки функциональности (в дальнейшем feature-ветка). Работа над задачами ведётся в отдельных ветках, которые по окончании вливаются в master. Когда master продвигается вперёд, простой git diff от вершины feature-ветки до её merge base уже недостаточен для отображения настоящего различия между этими ветками: он покажет разницу вершины feature-ветки лишь с одним из предыдущих состояний master.


Ветка master продвигается за счёт вливания новых изменений. Результат git diff master...alice/master не отражает этих изменений master.

Почему же невозможность увидеть эти изменения в ходе анализа pull request-а является проблемой? Тому есть две причины.

Конфликты слияния (merge conflicts)


С первой проблемой вы наверняка регулярно сталкиваетесь — конфликты слияния. Если в вашей feature-ветке вы измените файл, который в то же время был изменён в master, git diff по-прежнему будет отображать только изменения, сделанные вами в feature-ветке. Однако при попытке выполнить git merge вы столкнётесь с ошибкой: git расставит маркеры конфликтов в файлах вашей рабочей копии, поскольку сливаемые ветки имеют противоречивые изменения, — такие, которые git не в состоянии разрешить даже с помощью продвинутых стратегий слияния.


Конфликт слияния

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

Однако конфликты слияния — это меньшая неприятность, с которой вы можете столкнуться при использовании «triple dot» git diff для pull request-ов, по сравнению с другой проблемой: особый тип логического конфликта будет успешно слит, но сможет внести коварную ошибку в кодовую базу.

Логические конфликты, остающиеся незамеченными во время слияния


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

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

// flat fees and taxes
var customsFee          = 5.5;
var immigrationFee      = 7;
var federalTransportTax = .025;

function calculateAirfare(baseFare) {
    var fare = baseFare;                
    fare += immigrationFee;
    fare *= (1 + federalTransportTax);
    return fare;
}

Здесь содержится очевидная ошибка: автор забыл включить в расчёт таможенный сбор!

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

Алиса добавила строку для учёта customsFee перед immigrationFee:

function calculateAirfare(baseFare) {
    var fare = baseFare;                
+++ fare += customsFee; // Fixed it! Phew. Glad we didn't ship that! - Alice
    fare += immigrationFee;
    fare *= (1 + federalTransportTax);
    return fare;
}

Боб сделал аналогичную правку, однако поместил её после immigrationFee:

function calculateAirfare(baseFare) {
    var fare = baseFare;                
    fare += immigrationFee;
+++ fare += customsFee; // Fixed it! Gee, lucky I caught that one. - Bob
    fare *= (1 + federalTransportTax);
    return fare;
}

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

function calculateAirfare(baseFare) {
    var fare = baseFare;                
    fare += customsFee; // Fixed it! Phew. Glad we didn't ship that! - Alice
    fare += immigrationFee;
    fare += customsFee; // Fixed it! Gee, lucky I caught that one. - Bob
    fare *= (1 + federalTransportTax);
    return fare;
}

(Это, разумеется, надуманный пример, однако дублированный код или логика могут вызвать весьма серьёзные проблемы: к примеру, дыру в реализации SSL/TLS в iOS.)

Предположим, что вы сначала слили в master изменения pull request-а Алисы. Вот что показал бы pull request Боба, если бы вы использовали «triple dot» git diff:

function calculateAirfare(baseFare) {
    var fare = baseFare;                
    fare += immigrationFee;
+++ fare += customsFee; // Fixed it! Gee, lucky I caught that one. - Bob
    fare *= (1 + federalTransportTax);
    return fare;
}

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

На самом же деле, при анализе pull request-а вы хотели бы видеть, как master изменится после слияния изменений из ветки Боба:

function calculateAirfare(baseFare) {
    var fare = baseFare;                
    fare += customsFee; // Fixed it! Phew. Glad we didn't ship that! - Alice
    fare += immigrationFee;
+++ fare += customsFee; // Fixed it! Gee, lucky I caught that one. - Bob
    fare *= (1 + federalTransportTax);
    return fare;
}

Здесь явно обозначена проблема. Рецензент pull request-а, будем надеяться, заметит дублированную строчку и уведомит Боба о том, что код нужно доработать, и тем самым предотвратит попадание серьёзной ошибки в master и, в конечном счёте, в готовый продукт.

Таким образом мы решили реализовать показ изменений в pull request-ах в Bitbucket. При просмотре pull request-а вы видите, как на самом деле будет выглядеть результат слияния (т.е. фактически, результирующий коммит). Чтобы осуществить это, мы производим настоящее слияние веток и показываем разницу между получившимся коммитом и верхушкой целевой ветки pull request-а:


git diff C D, где D — это коммит, получившийся в результате слияния, показывает все различия между двумя ветками

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


Сравнение на основе коммита слияния, используемое в Bitbucket, показывает фактические изменения, которые будут применены, когда вы выполните слияние. Загвоздка в том, что этот алгоритм сложнее в реализации и требует значительно больше ресурсов для выполнения.

Продвижение веток


Во-первых, коммит слияния D на самом деле ещё не существует, а его создание — относительно дорогой процесс. Во-вторых, недостаточно просто создать коммит D и на этом закончить: B и C, родительские коммиты для нашего коммита слияния, могут поменяться в любое время. Мы называем изменение любого из родительских коммитов пересмотром (rescope) pull request-а, поскольку оно, по сути, модифицирует тот набор изменений, который будет применён в результате слияния pull request-а. Если ваш pull request нацелен на нагруженную ветку вроде master, он наверняка пересматривается очень часто.


Коммиты слияния создаются каждый раз, когда любая из веток pull request-а изменяется

Фактически, каждый раз когда кто-то коммитит или сливает pull request в master или feature-ветку, Bitbucket должен создать новый коммит слияния, чтобы показать актуальную разницу между ветками в pull request-е.

Обработка конфликтов слияния


Другая проблема при выполнении слияния для отображения разницы меджу ветками в pull request-е заключается в том, что время от времени вам придётся иметь дело с конфликтами слияния. Поскольку git сервер работает в неинтерактивном режиме, разрешать такие конфликты будет некому. Это ещё больше усложняет задачу, но на деле оказывается преимуществом. В Bitbucket мы действительно включаем маркеры конфликтов в коммит слияния D, а затем помечаем их при отображении разницы между ветками, чтобы явно указать вам на то, что pull request содержит конфликты:


Зелёные строки добавлены, красные — удалены, а жёлтые означают конфликт

Таким образом, мы не только заранее выявляем, что pull request содержит конфликт, но и позволяем рецензентам обсудить, как именно он должен быть разрешён. Поскольку конфликт всегда затрагивает, как минимум, две стороны, мы считаем, что pull request — это лучшее место для нахождения подходящего способа его разрешения.

Несмотря на дополнительную сложность реализации и ресурсоёмкость используемого подхода, я считаю, что выбранный нами в Bitbucket подход предоставляет наиболее точную и практичную разницу между ветками pull request-а.


Автор оригинальной статьи — Тим Петтерсен, участвовал в разработке JIRA, FishEye/Crucible и Stash. С начала 2013 года он рассказывает о процессах разработки, git, непрерывной интеграции и поставке (continuous integration/deployment) и инструментах Atlassian для разработчиков, особенно о Bitbucket. Тим регулярно публикует заметки об этих и других вещах в Twitter под псевдонимом @kannonboy.

Надеюсь, что эта статья оказалась интересной. Буду рад ответить на вопросы и комментарии.
Даниил Пенькин @detouched
карма
44,0
рейтинг 0,0
Java Developer
Реклама помогает поддерживать и развивать наши сервисы

Подробнее
Спецпроект

Самое читаемое Разработка

Комментарии (25)

  • –1
    Зачем усложнять-то? Лично я против merge-commit-ов в мастере, так что делаю примерно так:

    $ git remote add alice git://bitbucket.org/alice/bleak.git
    $ git checkout master
    $ git fetch alice
    $ git merge --ff-only alice/master < — если упадет из-за конфликта или старого пулл реквеста, то просим Алису переделать пулл реквест
    $ git diff origin/master < — смотрим вносимые изменения
    $ git rebase -i origin/master < — опционально, если хотим поменять коммиты Алисы, собрать их в один коммит например
    $ git push origin master < — готово
    • +3
      Зачем так усложнять?
      > git merge feature-branch
      и готово.
      А то желание не иметь мердж коммиты в мастере стоит вам потерей связей между правками и их авторами после ваших ребейзов.
      • +3
        Сам по себе rebase не меняет автора коммита, так что если у ревьювера есть право пушить коммиты с чужим именем, то все ок.

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

        Хотя дело вкуса конечно. Можно потратить какие-то усилия и иметь чистую историю, а можно и не тратить и не иметь — это не критично для разработки в конце концов.
        • 0
          Но при таком условии кто-то ведь всё равно сквошить должен: или создатель коммита, или тот, кто в Вашем сценарии мёржит в master, правильно? Так что это одинаково удобно (или неудобно), на мой взгляд.
          Кроме того, сквошить можно в любой момент, даже после ревью, прямо перед слиянием пулл реквеста, — он это спокойно переваривает.

          Что касается чистой истории, это вопрос, как Вы отметили, спорный. К тому же, git умеет фильтровать из неё merge коммиты.
          • 0
            А ещё можно мержить без фаст форворда. Тогда в ветке останутся все маленькие авторские коммиты, а в мастер они вольются одним коммитом, без необходимости сквоша.
            • 0
              Именно так и поступает алгоритм pull request-а, описанный в статье. Если интересно, я могу привести точную последовательность git операций, которую выполняет Bitbucket Server для показа диффа pull request-а и для его слива в целевую ветку.

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

              Однако задача pull request-а состоит не в том, чтобы организовать коммиты, а в том, чтобы аккуратно и достоверно отобразить, к какому состоянию целевой ветки приведёт сливание его содержимого в неё, и именно её аспекты рассматриваются в этой статье.
    • –3
      я не против merge-commit-ов в мастере, но тем не менее согласен с тем, что усложнено через чур

      git:(master) $ git pull
      git:(master) $ git checkout feature/alice
      git:(feature/alice) $ git rebase -i master < — догоняем мастер
      git:(feature/alice) $ git push -f
      • +10
        > git push -f

        а в это время где-то в далёкой-далёкой галактике

        vasya $ git pull
        vasya $ fuck
        • 0
          Nope. Всё норм, как правило никто не лезет в чужие фиче-бранчи. Rebase спаситель истории репозитория!
        • +3
          git:(feature/alice) $ git push -f в своей ветке
      • 0
        Делаю то же самое только чуть более идиоматично

        # Делаем ребейс при пулле по умолчанию. Выполняем лишь один раз
        git config pull.rebase true

        # Создаем рабочий бранч и указываем upstream
        git checkout -b my_feature_branch --track origin/master

        # Тянем изменения и автоматически делаем ребейс
        git:(my_feature_branch) git pull
    • +3
      Не могу согласиться с тем, что описанный Вами способ проще. Он, на самом деле, очень похож на описанный в статье процесс, за исключением того, что вместо merge-коммита Вы предлагаете разрешать только fast forward. В таком варианте невозможно отобразить, какие именно конфликты произошли, поскольку если они есть, merge вообще не происходит.

      Необходимость же повторения этого процесса в случае, если master тем временем изменился, никуда не девается. Это то, что в статье называется пересмотром пулл реквеста (rescope).

      Если же Вы имели в виду исключительно ручной процесс, то он не позволяет иметь более одного ревьюера (собственно, выполняющего эти команды), что является сильно урезанным пулл реквестом. Ну, и на более-менее живом проекте придётся делать много рутинных операций, чтобы догонять постоянно убегающий master.
    • –1
      Есть существенный минус при таком подходе. Код Алисы разрабатывался и тестировался на какой-то конкретой ревизии. Если заставить ее отребейзить код на текущий мастер, исчезает уверенность в том, что ее изменения работают. Эти изменения в мастере могли что-то изменить, что пагубно скажется на работоспобности кода.
      • 0
        Давайте тогда вообще ничего не ребейсить и не мержить, пусть те, кому нужна фича а, юзают ветку алисы, а кому фича б — ветку боба. А те неудачники, кому зачем-то нужно несколько фич, пусть ставят несколько копий приложения из разных веток. А для надёжности, чтоб они уж точно никак друг другу не помешали, сделаем в формате обрабатываемых данных для каждой ветки маленькие, но совершенно несовместимые друг с другом различия.
        • –1
          Спасибо, но git merge сохраняет историю изменений и возможность сделать git bisect по ней.
  • +9
    По названию статьи почему-то ожидал какой то коммит, который был максимально публично заплюсован/зазвезден/закомментин положительно или еще что в таком духе. Не выспался видать =)
    • +4
      Тот самый из бамблби?
      • 0
        Странно, а по моему, первым делом на ум должно приходить решение этого вопроса. Я уж думал, забодали их в конец и сделали-таки
  • –1
    del
  • 0
    Склонение английских word-ов по правилам русской грамматики — интересный трюк :)
    • +2
      Полагаю, в разговорной речи большинство так и говорит: «коммитить», «пушить», «пуллить» и т.д. Это, безусловно, неправильно, но и переходить на «зафиксировать», «протолкнуть», «затянуть» совсем не хочется. В общем, какой-то компромисс…
      • 0
        del
      • +1
        Вот смотрите, есть русское слово «фиксировать», оно есть в словаре, но как перевод «commit» в этом контексте — не очень привычно. Есть такое, тоже русское, слово «коммит», его нет в словаре, но все его знают и спокойно употребляют в разговорной речи. А есть английское слово «commit». Оно тоже хорошее, и его вполне можно употребить в статье на русском языке. Какой из этих трёх вариантов выбрать — дело автора. Меня просто смутила попытка впихнуть английские слова в русскую грамматику.
  • 0
    Странно что никто не дал ссылку https://github.com/theonion/fartscroll.js/pull/9 как пример… Лучшего пулл риквеста
    • 0
      странно, то вы дали эту ссылку, ведь пост вообще о другом.

Только зарегистрированные пользователи могут оставлять комментарии. Войдите, пожалуйста.