ベストプルリクエスト

比較的最近、私は幸運にもアトラシアンのBitbucket Serverの開発チームに加わりました（9月まではStashと呼ばれていました ）。 ある時点で、この製品がどのようにHabréでカバーされているのか気になりましたが、驚いたことに、それについてのメモがわずかしかありませんでした。



この点で、私はストーリーテラーとして自分自身をテストし、Bitbucketの技術的な側面に触れることにしました。 私はこの目標をまったく追求していないので、広告の試みとしての私の意図を考慮しないようお願いします。 この記事が読者の興味を引く場合、トピックを作成し、質問に答えてみてください。



まず、Tim Pettersen の記事「A better pull request」を翻訳して、それに割り当てられたタスクを最も効果的に解決するために、プルリクエストがどのように見えるべきかを説明します。

---

Gitを使用する場合は、おそらくプルリクエストも使用します。 それらは、分散バージョン管理システムの出現以来、何らかの形で存在していました。 BitbucketとGitHubが便利なWebインターフェースを提供する前は、プルリクエストは、アリスからの簡単な電子メールであり、リポジトリからいくつかの変更を取得するように求めていました。 価値がある場合は、いくつかのコマンドを実行してこれらの変更をメインマスターブランチにプッシュできます。

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

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

もちろん、見ずにAliceからの変更をmasterに含めることは良い考えではありません。結局、 masterには顧客に配信するコードが含まれているため、おそらく何が入るかを注意深く監視する必要があります。 マスターに変更を単に含めるよりも正しい方法は、最初にそれらを別のブランチにマージし、それらを分析してからマスターにマージすることです：

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

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

3ポイントの構文を持つ指定されたgit diffコマンド （以下「トリプルドット」git diff ）は、 アリス/ブランチブランチの最上部とそのマージベースとの間の変更を表示します。 本質的に、これらはまさにアリスがメインブランチに含めるように私たちに要求する変更です。





_{git diff master ... alice / masterは git diff AB と同等です}



一見すると、これはプルリクエストの変更をチェックする合理的な方法のように思えます。 実際、これを書いている時点では、これはまさにgitリポジトリのホスティングを提供するほとんどのツールでプルリクエストの実装に使用されている種類の比較アルゴリズムです。



それにもかかわらず、 トリプルドットgit diffを使用してプルリクエストの変更を分析する場合、いくつかの問題があります。 実際のプロジェクトでは、メインブランチは機能ブランチ（以下、 機能ブランチ ）とは大きく異なる可能性があります。 タスクの作業は別々のブランチで実行され、最後にmasterにマージされます。 masterが前方に移動すると、 機能ブランチの最上部からマージベースまでの単純なgit diffは、これらのブランチ間の実際の違いを表示するのに十分ではなくなります。以前の状態のmasterの 1つのみと機能ブランチの最上部の違いを表示します





_{masterブランチは、新しい変更の注入を通じて推進されています。 git diff master ... alice / masterの結果は、これらの変更をmasterに反映しません。}



プルリクエストの分析中にこれらの変更を確認できないことが問題なのはなぜですか？ これには2つの理由があります。

競合のマージ

おそらく最初の問題が定期的に発生します-マージの競合。 同時に機能も変更された機能ブランチのファイルを変更した場合、 git diffは機能ブランチで行った変更のみを表示します。 ただし、 git mergeを実行しようとすると、エラーが発生します：gitは作業コピーのファイルに競合マーカーを配置します。マージされたブランチには競合する変更があるため、gitは高度なマージ戦略を使用しても解決できないためです。





_{競合のマージ}



誰もマージの競合に対処したいとは思わないでしょうが、少なくともファイルレベルのロックをサポートしていないバージョン管理システムについては、バージョン管理システムが与えられています（多くの問題があります）。



ただし、マージの競合は、プルリクエストにトリプルドットgit diffを使用する場合、他の問題と比較して面倒なものではありません：特殊なタイプの論理的な競合は正常にマージされますが、コードベースに潜在的なエラーを引き起こす可能性があります。

合併中に気付かれない論理的な競合

開発者が異なるブランチの同じファイルの異なる部分を変更すると、そのような競合が発生する可能性があります。 場合によっては、個別に機能し、バージョン管理システムの観点から競合することなく完全にマージされるさまざまな変更が、一緒に適用されたときにコードに論理エラーを引き起こす可能性があります。



これはさまざまな方法で発生する可能性がありますが、最も一般的なのは、2人の開発者が2つの異なるブランチで同じエラーを誤って認識して修正する場合です。 以下の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; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

それには明らかな間違いが含まれています。著者は計算に関税を含めるのを忘れていました！



次に、2人の開発者、アリスとボブを想像してください。それぞれの開発者はこのエラーに気づき、ブランチ内の他の開発者とは無関係に修正しました。



アリスは、 immigrationFeeの 前にアカウンティングcustomsFeeの行を追加しました。

 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; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

これらの各ブランチで異なるコード行が変更されているため、両方をマスターにマージすると、次々に成功します。 ただし、 マスターには追加された両方の行が含まれるようになるため、顧客は通関手数料を2回支払うことになります。

 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; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

（もちろんこれは不自然な例ですが、重複したコードやロジックは非常に深刻な問題を引き起こす可能性があります。たとえば、 iOSでのSSL / TLSの実装に穴が空くなどです。）



最初にAliceのプルリクエストの変更をmasterにマージしたとします。 トリプルドット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; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

共通の祖先と比較して変更を分析しているため、マージボタンをクリックしたときに発生するエラーの脅威に関する警告はありません。



実際、プルリクエストを分析する場合、Bobブランチからの変更をマージした後、 マスターがどのように変化するかを確認したいと思います。

 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; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

これは明らかな問題です。 プルリクエストのレビュー担当者は、行の重複に気付き、コードをファイナライズする必要があることをボブに通知することで、重大なエラーがマスターに、最終的には最終製品に届かないようにします。



したがって、Bitbucketでプルリクエストの変更の表示を実装することにしました。 プルリクエストを表示すると、マージの結果が実際にどのように見えるかがわかります （つまり、実際の結果のコミット）。 これを実現するために、ブランチを実際にマージし、結果のコミットとターゲットプルリクエストブランチのトップとの違いを示します。





_{git diff CD （ Dはマージコミット）は、2つのブランチ間のすべての違いを表示します}



興味がある場合は、同じリポジトリを複数のホスティングサイトに配置して、比較アルゴリズムの違いについて説明します。

• トリプルドットgit diff比較を使用した GullHubプルリクエスト
• トリプルドットgit diff比較を使用したGitLabプルリクエスト
• マージコミット比較を使用した Bitbucketプルリクエスト

Bitbucketで使用されるマージコミットの比較は、マージ時に適用される実際の変更を示しています。 欠点は、このアルゴリズムを実装するのがより難しく、実行するのに非常に多くのリソースを必要とすることです。

支店プロモーション

まず、マージコミットDは実際にはまだ存在しておらず、その作成は比較的高価なプロセスです。 次に、コミットDを作成してそこで終了するだけでは十分ではありません。マージコミットの親コミットであるBとCはいつでも変更できます。 いずれかの親を変更すると、プルリクエストのマージの結果として適用される変更セットを本質的に変更するため、プルリクエストのスコープを変更すると呼びます。 プルリクエストがmasterのようなロードされたブランチをターゲットにしている場合、おそらく非常に頻繁にレビューされます。





_{プルリクエストブランチが変更されるたびに、マージコミットが作成されます}



実際、誰かがプル要求をマスターまたは機能ブランチにコミットまたはマージするたびに、Bitbucketは新しいマージコミットを作成して、プル要求のブランチ間の実際の違いを表示する必要があります。

競合処理のマージ

プルリクエストのブランチ間の違いを表示するためにマージを実行するときの別の問題は、マージの競合に対処する必要がある場合があることです。 gitサーバーは非インタラクティブモードで動作しているため、このような競合を解決する人はいません。 これはタスクをさらに複雑にしますが、実際には利点であることがわかりました。 Bitbucketでは、マージコミットDに競合マーカーを含め、ブランチ間の違いを表示するときにフラグを立てて、プルリクエストに競合が含まれていることを明示します。





_{緑の線が追加され、赤の線が削除され、黄色の線が競合を意味します。}



したがって、プルリクエストに競合が含まれていることを事前に明らかにするだけでなく、レビュー担当者がそれをどのように解決すべきかを議論することもできます。 競合は常に少なくとも2つの側面に影響するため、プルリクエストはそれを解決する適切な方法を見つけるのに最適な場所であると考えています。



実装の複雑さが増し、使用されるアプローチのリソース集約性にもかかわらず、私たちがBitbucketで選択したアプローチはプルリクエストブランチ間で最も正確で実用的な違いを提供すると考えています。

---

元の記事の著者であるTim Pettersenは、JIRA、FishEye / Crucible、およびStashの開発に参加しました。 2013年の初め以来、彼は開発プロセス、git、継続的な統合と展開、および開発者、特にBitbucket向けのAtlassianツールについて語っています。 ティムはこれらのことや他のことについてのメモを@kannonboyという仮名でTwitterに定期的に投稿しています。



この記事がおもしろいことを願っています。 質問やコメントにお答えできることを嬉しく思います。