LLVMコードレビューポリシーとプラクティス¶
LLVMのコードレビューポリシーとプラクティスは、プロジェクト全体の高いコード品質を維持するのに役立ちます。具体的には、私たちのコードレビュープロセスは、以下のことを目指しています。
可読性と保守性を向上させる。
堅牢性を向上させ、欠陥の導入を防ぐ。
提案された変更ごとに、他の貢献者の経験を最大限に活用する。
コミュニティリーダーによるメンターシップを通じて、新しい貢献者の成長と発展を支援する。
すべての貢献者が私たちのコードレビュープラクティスを理解し、コードレビュープロセスに参加することが重要です。
一般的なポリシー¶
レビューされるべきコードとは？¶
すべての開発者は、リポジトリにコミットする前に、重要な変更をレビューしてもらう必要があります。
コードはコミット前にレビューを受ける必要があるか？¶
コードは、コミット前またはコミット後にレビューできます。重要なパッチは、コミット前にレビューされることが期待されます。コミュニティ全体の合意要件（すべてのパッチ承認に適用されるもの）を満たす、より小さなパッチ（または開発者がコンポーネントを所有しているパッチ）は、明示的なレビューの前にコミットできます。不確実な状況では、パッチはコミット前にレビューする必要があります。
パッチを担当する開発者は、コミット後のレビュー中に要求された変更を含め、必要なすべてのレビュー関連の変更を行う責任も負うことに注意してください。
コードはコミット後にレビューできるか？¶
コミット後のレビューは推奨されており、以下で詳しく説明するツールを使用して実行できます。著者はコミット後のフィードバックに迅速に対応し、それに対処することが強く期待されています。そうしない場合は、パッチがリバートされる理由となります。
コミュニティメンバーが最近のコミットについて懸念を表明し、この懸念がコミット前レビュー中に議論を促すほど重要な場合（より詳細な設計議論の必要性を含む）、彼らは元の著者へのリバートを要求することができ、その著者はパッチを迅速にリバートする責任があります。開発者はしばしば意見が異なり、より多くのレビューを求める開発者の方を優先することで、ツリー内のコードに関する議論が長引くのを防ぎます。これはパッチ作成者の過失を示すものではなく、コミット後のレビュープラクティスに固有のものです。パッチをリバートすることで、他の開発を妨げることなく設計議論を行うことができ、懸念が解決されれば、パッチは実質的にそのまま再適用される可能性があります。
再コミットする前に、パッチは一般的にさらなるレビューを受ける必要があります。問題を特定したコミュニティメンバーは、レビューに積極的に関与することが期待されます。問題がビルドボットによって特定された場合は、ビルドボット上のものと同様のハードウェアにアクセスできるコミュニティメンバーがレビューに関与することが期待されます。
注意：コミット後のフィードバックの基準は、コミット前のフィードバックよりも高くありません。フィードバックの提供を不必要に遅らせないでください。ただし、コードがコミットされた後で、コミット前にコメントしていたであろうことに気づいた場合は、（以前に気づいていれば）いつでもそのフィードバックを提供してください。
とは言うものの、元の変更がコミットされてからかなりの時間が経過している場合は、元のコミットにコメントするよりも、問題を解決するために新しいパッチを作成する方が良い場合があります。たとえば、元のパッチの作成者は、もはやプロジェクトのアクティブな貢献者ではない可能性があります。
コードレビューにはどのツールが使用されますか？¶
コミット前のコードレビューは、GitHubでプルリクエストを使用して実施されます。GitHubのドキュメントを参照してください。
RFCが必要なのはいつですか？¶
一部の変更は、単なるコードレビューには大きすぎます。LLVM言語リファレンスを変更する必要がある変更（例：新しいターゲットに依存しない組み込み関数の追加）、Clangでの言語拡張の追加などは、まずプロジェクトの*-devメーリングリストでRFC（コメント依頼）メールが必要です。ユーザーやダウンストリームのコードベースに大きな影響を与える可能性のある変更については、レビュー担当者はコードレビューに進む前にRFCで合意に達することを要求できます。とは言うものの、最初のパッチを投稿すると、RFCに関する議論に役立つことがあります。
コードレビューのワークフロー¶
コードレビューは反復プロセスになる可能性があり、パッチがコミットされる準備ができるまで続きます。具体的には、パッチがレビューのために送信されると、コミットされる前に明示的な承認が必要です。黙認された承認を想定したり、期限付きでパッチへの異議を募ったりしないでください。
注意
レビュー以外の目的でプルリクエストを使用している場合（例：コミット前のCI結果、便利なWebベースのリバートなど）、PRにskip-precommit-approvalラベルを付けます。
すべてのレビュー担当者のフィードバックを確認する¶
レビュー担当者によるすべてのコメントは、パッチの作成者が確認する必要があります。作成者や他のレビュー担当者が、そうしない正当な理由を明確に説明できない限り（そして、レビュー担当者が同意する必要がある場合）、提案された変更がパッチの将来のリビジョンに組み込まれることが一般的に期待されます。新しいパッチがすべての未解決のフィードバックに対処していない場合、作成者は更新されたパッチを提供するときにそれを明示的に述べる必要があります。Webベースのコードレビューツールを使用している場合、このようなメモは「差分」の説明（コミットメッセージに使用される「差分リビジョン」全体の説明とは異なります）で提供できます。
コードレビューで変更を提案するが、その提案がこれほど強く解釈されることを望まない場合は、明示的にそう述べてください。
注意
レビュー担当者のコメントに対応した後、レビュー再リクエストを押して、プルリクエストをレビュー担当者の注意を引くようにします。
すべての人の時間を効率的に使うことを目指す¶
レビュープロセスの反復回数を制限することを目指します。たとえば、変更を提案するときに、作成者にコード内の他の場所で同様の変更を行わせたい場合は、作成者がすべての変更を一度に行えるように、要求された変更のセットを説明してください。パッチを承認する前に複数のステップが必要になる場合（例：分割、リファクタリング、特定のパフォーマンステストからのデータの投稿）、これらのステップを可能な限り事前に説明してください。これにより、パッチの作成者とレビュー担当者は時間を最も効率的に使用できます。
LGTM - パッチがどのように受け入れられるか¶
レビュー担当者がパッチを受け入れたときに、パッチはコミットが承認されます。これはほとんどの場合、「LGTM」（Looks Good To Meの略）というテキストを含むメッセージに関連付けられています。必要なのは、1人のレビュー担当者からの承認のみです。
無条件のLGTM（コミットの承認）を提供する場合、すべてのレビュー担当者からのすべての議論とフィードバックをレビューし、すべてのフィードバックが対処され、他のすべてのレビュー担当者が承認されたパッチにほぼ間違いなく満足することを保証するのはレビュー担当者の責任です。不明な場合は、レビュー担当者は条件付き承認（例：「LGTM、ただし@someone、@someone_elseを待ってください」）を提供する必要があります。また、特定のコミュニティメンバーがまだレビューしていなくても、レビューを希望する可能性が高い場合は、これを行うこともできます。
レビュー担当者が特定のコミュニティメンバーにレビューを依頼し、1週間後にそのコミュニティメンバーがまだ応答していない場合は、パッチにpingを送信する（文字通り、パッチに「Ping」という単語をコメントとして送信することを意味します）か、代わりに、元のレビュー担当者にさらなる提案を求めてください。
特に異議がある可能性があるが、まだ誰もそうしていない場合、最近投稿されたパッチを他の人がレビューすることを希望している可能性が高い場合は、条件付き承認（例：「LGTM、ただし他の人がレビューを希望する場合に備えて数日待ってください」）を提供することも礼儀です。承認が非常に迅速に受け取られた場合、パッチの作成者はコミットする前に待つことも選択できます（これは非自明なパッチには間違いなく礼儀正しいと考えられています）。特に私たちのコミュニティのグローバルな性質を考えると、この待機時間は少なくとも24時間である必要があります。週末や主要な祝日にも注意してください。
私たちの目標は、設計上の決定と重要な実装の選択に関してコミュニティの合意を確保することであり、パッチの全体的な承認を提供する場合のレビュー担当者の責任の1つは、そのような合意が存在することを合理的に確信することです。コミュニティを十分に知らない場合は、コミットに対する最終承認を提供すべきではありません。最終承認を提供するレビュー担当者は、LLVMプロジェクトへのコミットアクセス権を持っている必要があります。
すべてのパッチは、変更によって影響を受けるプロジェクトの分野の少なくとも1人の技術専門家によってレビューされる必要があります。
分割リクエストと条件付き承認¶
レビュー担当者は、パッチの特定の側面を独立したレビューのために個別のパッチに分割することを要求する場合があります。レビュー担当者は、作成者が特定の問題や懸念に対処するフォローアップパッチを提供することを条件にパッチを受け入れることもできます（ただし、コミットされたパッチはプロジェクトを壊れた状態のままにすべきではありません）。さらに、レビュー担当者は、作成者がコミットする前にいくつかの小さな更新のセットを適用することを条件にパッチを受け入れることができ、該当する場合は、レビュー担当者がそうすることは礼儀正しいです。
意図せずにレビューをブロックしない¶
パッチをレビューするが、レビュープロセスが承認でブロックされることを意図しない場合は、明示的にそう述べてください。礼儀として、すべてのレビュー担当者が満足するまでパッチのコミットを待つのが一般的であり、すぐにパッチを再度確認する予定がない場合は、レビューでその事実を伝えてください。
誰がコードをレビューできる/すべきか？¶
専門家ではない人もコードをレビューすべき¶
パッチをレビューするために、コードベースの特定の分野の専門家である必要はありません。コードの一部が何をしているのかを質問しても構いません。何が起こっているのかがわからない場合は、あなただけではない可能性が高いです。少数の人だけが十分に理解しているコンポーネントを持つことは、コミュニティの長期的な最善の利益にはならないことを覚えておいてください。追加のコメントやテストケースが役立つことがよくあります（テストケースでのコメントを求めるのも問題ありません）。
さらに、作成者は質問を、問題のコードの可読性を再検討する理由として解釈することを推奨します。構造的な変更や、追加のコメントが適切な場合があります。
もしあなたが LLVM コミュニティに初めて参加されるのであれば、このプレゼンテーションも役立つかもしれません: .. _LLVM への貢献方法, 2019 LLVM Developers’ Meeting Presentation: https://youtu.be/C5Y977rLqpw
新しいコントリビューターがコードベースの知識を深める良い方法は、コードレビューを行うことです。コードレビューを行い、承認の決定を他の人に明確に委ねることは全く問題ありません。
専門家はコードレビューを行うべきです¶
もしあなたが、提案されたパッチによって影響を受けるコンパイラの領域の専門家であるならば、コードレビューを行うことを強く推奨します。もしあなたが関連するコードのオーナーであり、他の専門家がパッチをレビューしていない場合、専門家によるレビューを手配するか、自分でレビューする必要があります。
コードレビュー、スピード、相互主義¶
特に大規模な機能の場合、コードレビューはあなたが期待するよりも時間がかかることがあります。パッチのレビュー時間を短縮するための一般的な方法は以下の通りです。
他の人のパッチをレビューする。あなたが助ければ、誰もがあなたのために同じことをしてくれるでしょう。善意が私たちの通貨です。
パッチを ping する。もしそれが緊急であれば、このパッチを適用することがあなたにとって重要である理由を説明し、数日ごとに ping してください。もし緊急でない場合は、一般的な礼儀としての ping レートは1週間です。あなたは他のプロの開発者の貴重な時間を求めていることを忘れないでください。
IRC で助けを求める。IRC の開発者は、直接あなたを助けるか、適切なレビュー担当者を教えてくれるでしょう。
パッチを複数の小さなパッチに分割し、それらを積み重ねていく。パッチが小さければ小さいほど、誰かがすぐに目を通してくれる可能性が高くなります。これを行う際には、シリーズ内の各パッチのタイトルに "[N/M]" (1 <= N <= M) を追加すると、順序があることと、その順序が明確になります。
開発者は、レビュー担当者と作成者の両方としてコードレビューに参加する必要があります。もし誰かが親切にあなたのコードをレビューしてくれたら、あなたも他の誰かのために恩返しをすべきです。誰でもパッチをレビューしてフィードバックを提供できますが、パッチの承認は上記のポリシーに従う必要があることに注意してください。