Gertit/コード レビュー/レビューを受ける
- 他の人のコードをレビューすることについて学ぶには、チュートリアルとコード レビュー ガイドを参照してください。
コードの変更をより速くレビューしてもらい、受理される可能性を高めるにはどうしたらいいでしょうか?
要件
- バグを修正したり、新しい機能を追加するために、あなたが変更したコードがあること。
- コーディング規約に従っていること。
- コミット前のチェックリストに従っていること。
- 開発者向けセキュリティ チェックリスト に従っていること。
- Gerrit で使用する 開発者アカウント を入手し、あなたのマシンへの Git/Gerrit のセットアップが済んでいること。
さて、あなたはパッチを作成して Gerrit にアップロードし、コードのレビューとマージ (コード ベースへの取り込み) を受けようと計画しています。
あなたのパッチ
範囲内かどうか事前に確認
あなたが提案したコードの変更がバグを修正するものではなく、代わりに新しい機能を導入するものである場合は、まずメンテナーに話をして、あなたのアイデアがプロジェクトの範囲内にあることと、提案した技術的アプローチが最適な解決策であることを確認してください。[1] これを行うことで、時間の節約と潜在的な失望を削減できます。
あなたの変更をテストする
変更内容をあなたの開発環境でテストして、コンパイル エラーやテストの失敗がないことを確認します。 何らかの理由でパッチをテストしていない場合は、Gerrit のコメントでその旨を明示してください。 コミット前のチェックリストを確認することを検討してください。
不完全な修正や新たなバグを発生させないようにしましょう。 あなたのパッチにもっと作業が必要だとわかっている場合は、Gerrit で「-2」としてレビューするか、コミット メッセージに「[WIP]」 (work in progress: 作業進行中) の接頭辞をつけるなどして、明示的にその旨を伝えてください。
小規模なパッチを書く
小規模で独立していて完全なパッチが、より受け入れられやすいのです。[2][3] パッチ内でレビューするファイルが多ければ多いほど、レビューにかかる時間と労力は大きくなり[4]、複数または数多くのレビューの繰り返しが必要になる可能性が高くなります。[5]
コミットが別々のファイル群に変更を加えるものであり、その間にあまり依存関係がない場合は、より小さな個別のコミットとしたほうがいいかもしれません。
さらに、パッチに不必要な変更 (例えば、コーディング スタイルの修正など) を含めないように注意してください。[1]
しかし、同じファイルを何箇所も変更するようなコミットの場合は、1 つの大きなコミットに束ねましょう (--amend または squash を使用)。
意味のあるコミット メッセージを書く
コミット メッセージでは内容と理由を説明すべきです。 何が問題点でしたか? それをどのように解決しましたか? 正しく動作することをテストする方法は? 詳細は Gerrit/コミット メッセージの指針 を参照してください。
また、コミット メッセージは必ず校正し、正しい綴りや句読点を使用するようにしましょう。
説明文書を追加
パッチに含まれる機能がエンド ユーザーや管理者の目に見えるようになる場合、関連する説明文書を更新または作成するようにしてください。[1] 詳細情報は 開発の方針#説明文書の方針 を参照してください。
リベースと変更を混同しない
リベースするときは、リベースのみです。 リベース以外の変更がリベースの変更セット内で行われた場合、それを見つけるためにさらに多くのコードを読み通さなければならず、明白ではありません。 実際に変更を行う場合は、変更内容を説明する Gerrit コメントを残し、コミットの要約が正確であることを確認して訂正します。
あなたの作業
Submit a Phabricator ticket
Create a task in Phabricator explaining the motivation (see Phabricator/Help). Patches without Phabricator tickets are harder to find reviewers for. To associate them, include a line in the commit message that says "Bug:" followed by the Phabricator ticket number.
テストの失敗やフィードバックへの対応
Gerrit の設定を確認し、メール通知が来ていることを確認してください。 あなたのコードが自動テストに失敗したり、既に何らかのレビューを受けたりした場合は、コメントや再投稿でそれに応答してください。
(自動テストが失敗した理由を見るには、Gerrit の「失敗」コメントのリンクをクリックし、失敗したテストの赤い点にカーソルを合わせ、ポップアップが表示されるまで待ち、「コンソール出力」をクリックします。)
フィードバックには通常 (常にではありませんが)、Gerrit から C-2、C-1、C+1、C+2 のフラグが付きます。 これらが何を意味するか (レビュアーの視点から) については、Gerrit/コード レビュー#レビューを完了する で詳しく説明されています。 レビュアーの中には、新人投稿者を安心させるために、明示的な C-1 評価を用いずに改善点を指摘する人もいます。 それでも彼らの提案を真摯に受け止めるべきでしょう。
時には、あなたが無関係と認識するようなレビュー、例えば単なる表面的なレビューが届くこともあります。 そのようなレビューを無視するのではなく、些細な要望を満たすためにパッチを修正しましょう。あなたは反対するかもしれませんが、議論するには譲歩するよりも時間を消費します。
コード レビューとは、選挙に勝つことではなく、合意形成の問題です。 あなたは、否定的なフィードバックに対処することが期待されており、肯定的なレビューをさらに求めて「反対票を投票」(outvote) することはできません。 ネガティブなフィードバックに対処するためには、必ずしもパッチを変更する必要はありません - 時には、必要なのはよりよい説明だけです。 しかし、そのような場合でも、コミット メッセージやコード内のコメントにその説明を盛り込むことで、同じ悩みを持つ将来のメンテナーによりよい情報を提供できます。
一般的に: 我慢をして成長してください。 より経験豊富なパッチ執筆者は、より早く、より肯定的な反応を得ることができます。[5]
レビュアーを追加
レビュアーの選択は、レビュー時間に重要な役割を果たします。 より活発なレビュアーが、より迅速な反応を提供します。[5]
コミットした直後に、変更セットに 1 人か 2 人の開発者をレビュアーとして追加してください。 (これらはリクエストです。Gerrit では特定の 1 人にレビューを割り当てる方法はありません。) 経験豊富な開発者はこれを支援すべきです: 未レビューの変更セットが残っていることに気づいた場合は、レビュアーを追加してください。 レビュアーを見つける方法は以下の通り:
- コードのその部分を現在メンテナンスしている人、またはメンテナンスのトレーニングを受けている人を見つけるには、メイン メンテナー一覧、または拡張機能のページで列挙されているメンテナーを確認してください。
- Gerrit パッチの「プロジェクト」行にある虫眼鏡をクリックします。 次に、そのリポジトリで他の変更セットを探します。それらの変更セットを書いたりレビューしたりする人は、レビュアーとして追加するのにいい候補になるでしょう。 また、パッチを承認できる人を確認するには、上部ナビゲーション バーの「アクセス」をクリックし、「所有者」の行にあるリンクをクリックし、名前の一覧を表示します。
- 他のコミット要約や変更セットを検索できます。 リポジトリ ツリーをあなたのリポジトリやディレクトリに遷移し、「履歴を閲覧」をクリックすることで、例えば MediaWiki コアのデータベースでの変更など、その分野で誰が活動しているかを確認できます。 または Gerrit で検索してください: Matma Rex と Foxtrott はフロントエンドの変更のレビューに興味があるため、「message:css」を検索してコミット要約で CSS に言及している変更セットを見つけることで、その方たちを追加できます。 これと正規表現を使用して、あなたが扱っていると同じコンポーネントを扱っている変更を見つけて、レビューしそうな人を見つけることができます (検索の説明文書)。
さらにレビューする
たくさんの目で見ることでバグが見つかりやすくなります。 コード レビュー ガイドを読み、他の作者のコミットを褒めたり批評したりすることで支援しましょう。 コメントは、拘束力を持たず、マージや却下の原因にはならず、コード レビューに正式な影響を与えません。 しかし、レビューすることで学び、評判が上がり、将来あなたのコード変更案をレビューすることで恩返しをする人が現れるでしょう。 "How to review code in Gerrit" has the step-by-step explanation.
想定される障害への対応
タイムリーなフィードバックがない
フリー ソフトウェアやオープン ソース ソフトウェアのプロジェクトでは人手が限られており、開発者の関心も変化する可能性があります。 コード リポジトリの中には、より活発でメンテナンスされているものもあり、より迅速なレビューを受けられます。 また、保守管理者が不明確な場所や、廃墟のような場所もあり、長い時間待たされることもあります。
コード リポジトリの最新の活動は、あなたのローカル チェックアウト内で git log で確認できます。
放棄されたプロジェクトを引き継いでメンテナーになるには、こちらの手順に従ってください。
あなたのパッチが長い間気づかれずにいると思う場合、#wikimedia-tech 接続 IRC チャンネルで遠慮なく問題提起してください。
手直しや却下のその他の理由
すべての推奨事項に従ったとしても、パッチは手直しが必要な場合があります (まれに却下される場合もあります)。
既に述べたこと以外にも、より単純で効率的な方法がある場合の最適でない解決策、パフォーマンスの問題、セキュリティの問題、(変数などの) 名前付けの改良、既存のコードとの統合の衝突、作業の重複、API の意図しない (誤った) 使用、内部 API の変更案がリスクが高すぎると見なされた、などの拒否の理由 (すべてが同様に決定的なものとは限らない) は潜在的に存在します。[1]
判断の不一致に注意してください: パッチのレビュアーは、テストの失敗、不完全な修正、新しいバグの発生、最適でない解決策、矛盾した説明文書などを、パッチを却下する決定的な要因として考えることが、パッチの作成者よりも多いのです。[1]
関連項目
- 「コード レビューを早く受けるための 5 つのコツ」
- 「コード レビュー会議メモ」 - 2012年以降、関連する部分は現在の説明文書に含まれています
出典
- ↑ 1.0 1.1 1.2 1.3 1.4 Yida Tao; Donggyun Han; Sunghun Kim, "Writing Acceptable Patches: An Empirical Study of Open Source Project Patches," in Software Maintenance and Evolution (ICSME), 2014 IEEE International Conference on , vol., no., pp.271-280, Sept. 29 2014-Oct. 3 2014
- ↑ Peter C. Rigby, Daniel M. German, and Margaret-Anne Storey. 2008. Open source software peer review practices: a case study of the apache server. In Proceedings of the 30th international conference on Software engineering (ICSE '08). ACM, New York, NY, USA, 541-550.
- ↑ Peter Weißgerber, Daniel Neu, and Stephan Diehl. 2008. Small patches get in!. In Proceedings of the 2008 international working conference on Mining software repositories (MSR '08). ACM, New York, NY, USA, 67-76.
- ↑ Amiangshu Bosu, Michaela Greiler, and Christian Bird. 2015. Characteristics of useful code reviews: an empirical study at Microsoft. In Proceedings of the 12th Working Conference on Mining Software Repositories (MSR '15). IEEE Press, Piscataway, NJ, USA, 146-156.
- ↑ 5.0 5.1 5.2 Baysal, O.; Kononenko, O.; Holmes, R.; Godfrey, M.W., "The influence of non-technical factors on code review," in Reverse Engineering (WCRE), 2013 20th Working Conference on , vol., no., pp.122-131, 14-17 Oct. 2013