これらの一般的なプロセスに従う限り、コードレビューは怖くありません。
プロジェクト全体を完全に理解する前に、コードをレビューする必要がありますか？それとも、どう進めていいかわからないと思われないように、レビューを避けているのでしょうか？
この記事では、より良い方法を紹介したいと思います。 すべてを知っている必要はありません。実際、私の個人的な経験では、それは非常に一般的なことです。
インターンとして入社したとき、コードレビューに参加するよう求められたことを覚えています。1か-1の投票システムがあり、最初はレビューするのをためらうことがよくありました。ある変更に対して+1票を投じて、他の誰かが-1票を投じたら、私はバカに見えるのだろうかと自問自答していました。
あなたが＋1票を投じ、他の人が-1票を投じた場合、それは何を意味するのでしょうか？答えは、何の意味もありません！あなたは、他の誰かが気づいた細部を見逃しただけかもしれません。世界の終わりを意味するものではありません。これが投票システムが使われる理由です。すべてのオープンソースプロジェクトと同様に、コードのマージは共同作業です。
最近、コードレビューの数が多すぎて、全部こなすのがやっとです。また、レビューに参加するコントリビューターの数が確実に減っていることにも気づきました。
このため、コードレビューに関する私の個人的な見解を記事にしたいと思います。この記事では、いくつかのヒントとコツを紹介します。自分自身に問いかけるべきいくつかの質問と、コードをレビューするときに注意すべき点を紹介します。
コードレビューの目的は何ですか？
とても簡単なパッチを書いたことがありますか？あなたはそれがとても些細なもので、レビューされる必要はないと思ったでしょう。たぶん、ただマージしただけでしょう。その日の深夜になるまで、あなたは明らかな、あるいは愚かなミスを犯していることに気づきました。例えば間違ったインデントや、関数を呼び出す代わりに数行の重複したコードを書いてしまったような。
もし他の誰かがコードを見直すとしたら、このようなことを見つけるでしょう。
コードレビューの目的の一つは、解決しようとしている問題に新鮮な目と新しい視点をもたらすことです。この新しい文脈こそが、コードレビューが非常に重要な理由です。
他人のコードやプロジェクト、あるいはその両方をレビューするには、言語の専門家でなければならないと思うかもしれません。すべてのコードレビュアーがあなたに伝えたい秘密を教えましょう！大きな間違いです！プロジェクトやプログラミング言語を完全に理解しなくても、変更に対して新鮮な視点を提供することはできます。以下に、コードレビューの一般的なプロセスを紹介します。
コードレビューの一般的なプロセス
その変更、その変更が解決しようとしている問題、その変更を行う理由の理解
なぜ変更が必要なのかの説明と、関連する背景があれば、メッセージに記載すべきです。もしそうでなければ、関連する情報が提供されるまで、要求して-1票を投じてください。
その変更が解決しようとしている問題は、解決する必要があるのか？プロジェクトが注力すべきことなのか、それともプロジェクトとはまったく無関係なことなのか？
あなたならどのように解決策を実行しますか？それは違いますか？
この時点で、あなたはすでに何のためのコード変更なのかを知っているはずです。あなたの立場ならどうしますか？変更の詳細なレビューを進める前に、このことを考えてください。もしあなたが別の解決策を思いつき、それがより良いと思うなら、レビューでそれを発表してください。なぜその方向に行かなかったのか、作者に聞いてみてください。
コードは変更あり、変更なしで実行されます。
私は通常、コードにいくつかのブレークポイントを設定し、コードを実行し、新しいコードが他のコードとどのように相互作用するかをチェックします。
コード全体を実行できない場合は、新しいコードを含む関数を新しいローカルファイルにコピーし、入力データをシミュレートしてから実行してみてください。これは、プロジェクト全体を実行する方法がわからない場合や、実行に必要な特別な環境にアクセスできない場合に便利です。
新しいコードは何かを壊していますか？
つまり、何でもいいんです。起こりうる結果を考えてください。
例えば、新しいコマンドラインオプションは、常にターゲットに受け入れられるのでしょうか？
新しいオプションが受け入れられない、あるいは他の何かと衝突するような状況がありますか？
新しいコードは何か新しいものをインポートしているのかもしれません。では、この新しいライブラリーや、おそらく新しい依存関係は、古いバージョンやプロジェクトのランタイム・システムで見つけることができるのでしょうか？
セキュリティについてはどうですか？新しい依存関係は十分安全ですか？少なくとも、ネットで簡単に検索することはできるでしょう。また、コンソールログの警告にも注意してください。同じライブラリの中に、より安全な関数が見つかることもあります。
新しいコードは有効ですか？
提案されている解決策がおそらく正しいことを確認しました。次はコードそのものをチェックする番です。コードの妥当性と必要性に注目する必要があります。
新しいコードのスタイルをチェックしてください。プロジェクトのコードスタイルに合っていますか？どのようなオープンソースプロジェクトにも、そのプロジェクトが守っているスタイルとグッドプラクティスを貢献者に知らせる文書があります。
例えば、OpenStack コミュニティのすべてのプロジェクトには HACKING.rst ファイルがあります。また、新規コントリビューターガイド（New Contributor Guide）には、必ず知っておくべき情報がすべて記載されています。
以下は、システムに追加された変数とインポートのリストです。
あなたがレビューしているコードは、何度か繰り返されていることが多く、最終的なバージョンは最初のバージョンと大きく異なることもあります。そのため、以前のバージョンで追加された変数や参照を忘れがちです。自動検出は、Pythonの[flake8][12]に似たlintツールでよく行われます。
新しい変数を宣言せずにコードを書き換えることはできますか？通常は可能ですが、問題はその方が良いかどうかです。その方が得なのでしょうか？目標は、できるだけ多くの1行のコードを作ることではなく、効率的で読みやすいコードを書くことです。
新しい機能や方法は必要ですか？
プロジェクト内の他の場所に、再利用できる同様の機能を持つ関数がありますか？ライブラリの再発明や、すでに定義されているロジックの再実装を避けることは、常に意味のあることです。
ユニットテストはありますか？
パッチが新しい関数を追加したり、関数の中に新しいロジックを追加したりする場合、それに対応するユニットテストも一緒に提供されるべきです。新しい関数の作者は、誰よりもその関数のユニットテストを書くことができる立場にあります。
リファクタリングの検証
もしこのコミットが既存のコードをリファクタリングするのであれば、自問してみてください：
- これは削除できますか？安定版ブランチに影響はありますか?
- すべての出演は削除されたのですか？
grep コマンド 検索できます。このせいで何度-1に投票したことか。誰にでもできる単純なミスなので、誰でも見つけることができます。
提出物のオーナーがこうしたことを見落としがちなのは、まったく理解できることです。私もこのミスを何度も犯しました。結局、問題の根本は、レビューをアップすることに熱中するあまり、リポジトリ全体をチェックすることを忘れていたことだと気づきました。
プロジェクトのリポジトリをチェックすることに加えて、他のコード・ユーザーをチェックすることも不可欠です。このプロジェクトをインポートした他のプロジェクトがあれば、それらもリファクタリングする必要があるかもしれません。OpenStackコミュニティには、他のコミュニティプロジェクトをチェックするためのツールがあります。
プロジェクト文書に変更が必要ですか？
もう一度、 、問題のコード変更がプロジェクトのドキュメントに記載されているかどうかをチェックすることができます。その変更がエンドユーザーに知らせるために文書化する必要があるのか、それともユーザーエクスペリエンスに影響を与えない単なる内部的な変更なのか、常識を働かせて判断してください。
おまけのヒント：思慮深く
最後に一言
唯一の悪いレビューは、レビューがないことです。審査と投票では、あなたは意見を提供し、それに対して投票します。あなたが最終的な決定を下すとは誰も期待していませんが、投票システムによって、あなたは視点や意見を提供することができます。私を信じてください。
