コードレビューガイド

このドキュメントは、コードレビューに関する XLA チームの見解について説明することを目的としています。オープンソースプロジェクト全般、特に XLA に取り組んできた長年の集合的経験から育った見解です。

オープンソースプロジェクトが異なれば、レビュー担当者がコード作成者にどれだけ要求できるかについての文化的期待も異なります。一部のプロジェクトでは、レビュー担当者が「ほぼ正しい」プル リクエスト（PR）を受け取り、それを自分で変更して送信します。XLA はこれとは逆のアプローチをとっており、追加の変更なしで提出できるほど良くなるまで、作成者は PR をイテレートすることが期待されています。

このアプローチには主に、PR 作成者に本格的な XLA 貢献者になることを学んでもらいたいという理由があります。レビュー担当者が PR の問題を自分で修正すると、作成者が学習するのがはるかに難しくなります。 XLA のアプローチは、レビュー担当者とコード作成者の両方にとって困難な場合がありますが、最終的にコミュニティの成長に役立つと信じています。

「本格的な XLA 貢献者」になることは、バグのないコードを書くことだけではありません。「XLA の変更方法」については、学ぶべきことがさらに多くあります。これには、以下の内容が含まれます。

• コーディングスタイル

• 注意すべきエッジケース

• テストの記述に関する期待

• コメントと PR の説明に関する期待

• および変更内容をサポートするインフラストラクチャの確立に関する期待

プロジェクトの知識とレビュー担当者からの信頼を積み上げるにつれ、コメントの数が減少することを期待できます。自然に、レビュー担当者の期待に合ったコードを書くようになるためです。

多くのオープンソースプロジェクトと同様に、XLA には経験が非常に豊富な人が数名と比較的新しい人が多数います。経験豊富な私たちは、多くの時間を要求されます。PR をタイムリーに進め続けるためには、レビュー担当者が必要とする時間と必要なイテレーションの回数を削減できるように、次のことを行うことができます。

• PR を送信する前に、慎重に検討するか、同僚に PR をレビューしてもらう。 レビュー用に PR を送信する前に、些細なミス（コード スタイル、スペル、文法ミスなど）を可能な限り排除するようにしてください。すべてのテストに必ず合格するようにします。

• レビュー担当者のコメントをよく読んでください。 新しいバージョンをプッシュする前に、レビュー担当者が何を求めているかを理解し、すべてのコメントに対応するように努めます。

• 「自転車置き場の議論」は避けましょう。 技術的な議論や意見の相違は非常に価値があり、完璧な人は誰もいませんが、違いを生まない、または単なるスタイル上の議論は避けるようにしてください。レビュー担当者のコメントに同意できない場合は、その理由をできるだけ正確かつ包括的に詳述して、長い議論を避けるようにします。

• 以下にリストされている「よくあるレビューの質問」を避けましょう。 以下に、よくある質問に対するいくつかの回答とその根拠を記載しています。

一般に、PR のレビューにかかる時間をできるだけ短くするように努めてください。そうすれば、変更をすぐに確認したいと思うでしょう。

XLA に貢献していただきありがとうございます！ハッキングをお楽しみください！

よくあるレビューの質問

「このインフラストラクチャの変更は私のPRとは関係ありません。なぜ私が担当するのですか？」

XLA チームにはインフラストラクチャ専門のチームがありません。そのため、チームでヘルパーライブラリをビルドし、技術的負債を回避しなければなりません。これは、XLA への通常の変更作業の一環であると考えているため、全員の参加が期待されています。通常、コードを書くときに必要に応じてインフラストラクチャを構築しています。

XLA レビュー担当者は、あなたが書いた PR に加えて、何らかのインフラストラクチャを構築する（または PR に大きな変更を加える）ように依頼する場合があります。このリクエストは、行おうとしている変更に対して不必要またはそれとは直交しているように見える場合もありますが、これは、構築する必要があるインフラストラクチャの量に関するあなたの期待とレビュー担当者の期待に不一致があるためである可能性があります。

期待が同じでないことは問題ではありません！これは、プロジェクトを始めたばかりの場合に想定されることです（経験豊富な私たちでもたまに起こることがあります）。過去に取り組んだプロジェクトでは、異なる期待があったことも考えられます。それも問題ではなく、想定内のことです。いずれかのプロジェクトのアプローチが間違っているわけでなく、別のプロジェクトというだけのことです。このプロジェクトに期待されていることを学ぶ機会として、他のすべてのレビューコメントとともにインフラのリクエストも受け付けることをお勧めします。

「コメントの内容は今後の PR で対応していいですか？」

PR のインフラストラクチャリクエスト（またはその他の大規模なリクエスト）に関してよくある質問は、変更を元の PR で行う必要があるかどうか、または今後の PR でフォローアップとして行うことができるかどうかです。

XLA では一般に、PR 作成者がフォローアップ PR としてレビュー コメントに対応することを許可していません。レビュー担当者が、この PR で対応する必要があると判断した場合、大規模な変更リクエストであっても、通常、作成者が元の PR で対応することを期待しています。これは、Google の外部だけでなく内部にも適用されている基準です。

XLA がこのアプローチを採用しているのには、いくつかの理由があります。

• 信頼: レビュー担当者の信頼を得ることは重要な要素です。オープンソースプロジェクトでは、貢献者は意のままに現れたりいなくなったりしがちです。PR を承認した後、レビュー担当者には、約束されたフォローアップが実際に行われたかどうかを確認する手段がありません。

• 他の開発者への影響: XLA の特定の部分に触れる PR を送信した場合、他の人が同じ箇所を見ている可能性が高くなります。PR で技術的負債を受け入れる場合、フォローアップが提出されるまで、このファイルを見ているすべての人がこの負債の影響を受けることになります。

• レビュー担当者の帯域幅: フォローアップへの変更を延期すると、すでに負荷が高くなっているレビュー担当者に様々なのコストがかかります。レビュー担当者は、フォローアップを待っている間にこの PR の内容を忘れてしまい、次のレビューが困難になります。また、レビュー担当者は、待機中のフォローアップを追跡し、実際にフォローアップが行われるようにする必要も生じます。他のレビュー担当者がレビューできるように、元の PR と真に直交するような変更を行うことができれば、帯域幅の問題は少なくなるかもしれませんが、私たちの経験では、これはめったに起こることではありません。