オープンソースコードレビューの10の一般的なステップ

プロジェクト全体を完全に理解する前に、コードをレビューする必要がありますか？それとも、無知だと思われないようにレビューを避けていますか？

この記事の目的は、より良い方法であるコードレビューを紹介することです。

インターンとして Red Hat に入社したことを覚えています。

ある変更に対して+1票を投じたのに、他の誰かが-1票を投じた場合、それは何を意味するのでしょうか？答えは、何の意味もありません！もしかしたら、他の誰かが気づいていた細かい点を見逃していただけかもしれません。世界が終わるわけではありません。だからこそ、私たちは投票システムを採用しているのです。他のオープンソースプロジェクトと同様に、コードのマージは共同作業なのです。

最近、コードレビューの依頼があまりにも多くて、もう圧倒されてしまいそうです。また、レビューに参加してくれる貢献者の数が着実に減っていることにも気づきました。

そのため、コードレビューに関する私の個人的な見解をまとめた記事を書きたいと思いました。この記事では、コードレビューを行う際のヒントやコツをいくつかご紹介します。また、コードレビューを行う際に自問自答すべき質問や留意すべきポイントについてもご紹介します。

コードレビューの目的は何ですか?

非常にシンプルなパッチを書いたことがありますか？とても些細なことなのでレビューは不要だと思ったかもしれません。もしかしたら、そのままマージしてしまったかもしれません。しかし、後になって、インデントの誤りや、関数を呼び出す代わりに数行の重複コードを書くなど、明らかなミスや些細なミスを犯していたことに気づいたのです（そう、これらはすべて教訓です！）。

誰かがコードをレビューすれば、これらのことが見つかるでしょう。

コードレビューの目的の一つは、新たな視点を与え、解決しようとしている問題を新たな角度から見ることができるようにすることです。この新たな視点こそが、コードレビューが極めて重要である理由です。

他人のコードやプロジェクト、あるいはその両方をレビューするには、言語のエキスパートでなければならないと思っているかもしれません。しかし、コードレビュー担当者なら誰もが知りたい秘密を教えましょう。それは全くの間違いです！変更点について新鮮な視点を提供するために、プロジェクトやプログラミング言語を完全に理解する必要はありません。以下では、一般的なコードレビューのプロセスをご紹介します。

コードレビューの一般的なプロセス

これは私のコードレビューのプロセスであり、いくつかの重要なポイントに分かれています。このプロセスには、コードの変更とその影響に焦点を当てるために私が自問自答する質問が含まれています。この順序に厳密に従う必要はありません。何らかの理由で実行できない手順がある場合は、その手順をスキップしてください。

1. 変更点、変更によって解決しようとしている問題、変更がなぜこのように行われるのかを理解します。

変更が必要な理由の説明と関連する背景を、コミットメッセージに含める必要があります。

変更によって解決しようとしている問題は、本当に解決する必要があるのでしょうか？それはプロジェクトで対処すべき問題でしょうか、それともプロジェクトとは全く関係のない問題でしょうか？

2. このソリューションをどのように実装しますか？何か違いがあるでしょうか？

ここまでで、コード変更の目的はすでにお分かりでしょう。このような状況で、あなたならどうしますか？変更点の詳細なレビューを行う前に、この質問について考えてみてください。もし別の解決策を思いつき、自分の解決策の方が優れていると確信できるなら、レビューでその点を指摘してください。-1票を投じる必要はありません。なぜその方向に進まなかったのか、オーサーに尋ね、この議論がどこへ向かうのかを確認しましょう。

3. 変更したコードと変更していないコードを実行します。

通常、コードにいくつかのブレークポイントを設定し、コードを実行して、新しいコードが残りのコードとどのように相互作用するかを確認します。

コード全体を実行できない場合は、新しいコードを含む関数を新しいローカルファイルにコピーし、入力データをシミュレートしてから実行してみてください。これは、プロジェクト全体の実行方法がわからない場合や、実行に必要な特定の環境にアクセスできない場合に役立ちます。

4. 新しいコードによって何かが壊れますか?

何でもいいんです。起こりうる結果について考えてみてください。

たとえば、新しいコマンドラインオプションは常にターゲットで受け入れられるでしょうか?

新しいオプションが受け入れられない、または他のものと競合する状況はありますか?

新しいコードは何か新しいものをインポートしているかもしれません。その場合、この新しいライブラリ、そしておそらく新しい依存関係は、古いバージョンまたはプロジェクトのランタイムシステムで見つかるでしょうか？

セキュリティはどうでしょうか？新しい依存関係は十分に安全でしょうか？少なくともオンラインで簡単に検索することはできます。また、コンソールログの警告にも注意してください。同じライブラリ内に、さらに安全な関数が見つかることもあります。

5. 新しいコードは有効ですか?

提案された解決策が正しい可能性が高いことが確認できました。次はコード自体を検証しましょう。コードの妥当性と必要性に焦点を当てる必要があります。

新しいコードのスタイルを確認してください。プロジェクトのコーディングスタイルと一致していますか？オープンソースプロジェクトには、（新しい）貢献者にプロジェクトが従うスタイルとベストプラクティスを知らせるドキュメントが（あるべきです）含まれているべきです。

例えば、OpenStackコミュニティのすべてのプロジェクトにはHACKING.rstファイルがあります。また、重要な情報がすべて含まれた新しいコントリビューターガイドもよく見つかります。

6. 新しく追加されたすべての変数とインポートが使用されていることを確認します。

レビュー対象のコードは多くの場合、複数回のイテレーションを経ており、最終版が最初のバージョンと大きく異なることがあります。そのため、以前のバージョンで追加された変数や参照を忘れてしまうことがよくあります。自動検出には通常、Pythonの[flake8][12]に似たlintツールが使用されます。

（LCTT翻訳者注：lintとは、プログラミング言語において、コードの潜在的なエラーを発見し、コーディングスタイルを制約するために用いられるツールを指します。C言語における静的解析ツールlintに由来しています。「lint」という言葉は、もともと衣服に付着した糸くずや埃を意味しており、「lint」という名前は、プログラミング中に発生する「糸くずや埃」を捉えることを意図しています。）

(LCTT 編集者注: 「Lint」ツールは「コードクリーンアップ」または「コードクリーニング」ツールと翻訳できると思います。)

新しい変数を宣言せずにコードを書き換えることはできますか？基本的には可能ですが、問題はそうした方が良いかどうかです。そうすることでどのようなメリットがあるのでしょうか？私たちの目標は、できるだけ多くの単一行のコードを作成することではなく、効率的で読みやすいコードを書くことです。

7. 新しい機能やメソッドは必要ですか?

プロジェクトの他の場所に、同様の再利用可能な関数はありますか? 車輪の再発明や、既に定義されたロジックの再実装は避けるべきです。

8. ユニットテストはありますか?

パッチによって新しい関数や関数内に新しいロジックが追加される場合は、対応するユニットテストも含める必要があります。新しい関数の作成者は、他の開発者よりもユニットテストの作成に長けています。

9. リファクタリングを検証する

このコミットが既存のコードをリファクタリングした場合（変数の名前変更、変数のスコープの変更、パラメータの追加または削除による関数のフットプリントの変更、何かの削除など）、次の点を自問してください。

これは削除できますか？安定したブランチに影響しますか？
すべてのインスタンスが削除されましたか?

`grep` コマンドを使えば見つけることができます。このせいで、私が何度 -1 をキャストしたか、信じられないほどです。これは誰でも犯しがちな単純なミスなので、誰でもすぐに見抜くことができます。

所有者がこうした点を簡単に見落としてしまうのは当然です。私自身も何度もこのミスを犯しました。最終的に、問題の根本原因は、レビューに提出することに熱心になりすぎて、リポジトリの完全なレビューを忘れてしまったことにあることに気づきました。

プロジェクトリポジトリの確認に加えて、他のコードユーザーを確認することも重要です。他のプロジェクトがこのプロジェクトをインポートしている場合は、それらもリファクタリングする必要があるかもしれません。OpenStackコミュニティには、他のコミュニティプロジェクトを照会するためのツールがあります。

10. プロジェクト文書を変更する必要がありますか?

`grep` コマンドを再度使用して、関連するコード変更がプロジェクトのドキュメントに記載されているかどうかを確認できます。この変更をエンドユーザーに通知するためにドキュメント化する必要があるのか、それともユーザーエクスペリエンスに影響を与えない内部的な変更なのかを、常識的に判断してください。

追記：思慮深い検討

新しいコードをレビューした後に提案やコメントをする際は、思慮深く、正確なフィードバックを提供し、詳細を説明してください。理解できない点があれば質問してください。コードに誤りがあると思われる場合は、その理由を説明してください。作成者が何が間違っていたのか理解していなければ、修正することはできないことを忘れないでください。

最後の数文

レビューの唯一の欠点は、レビューが全くないことです。レビューと投票を通して、あなたは自分の視点を提示し、票を投じることができます。誰もあなたに最終決定を下すことを期待していません（コアメンテナーでない限り）。しかし、投票システムでは、あなたの意見や視点を提示することができます。信じてください、パッチオーナーはあなたがそうしてくれたことをとても喜ぶでしょう。

DUICUO