洗練されたコードレビュー
2021年1月28日
人々はコードレビューについて考える際、通常は開発チームのワークフローにおける明示的なステップとして考えます。今日では、プリインテグレーションレビュー(プルリクエストで行われる)がコードレビューの最も一般的なメカニズムであり、プルリクエストを使用しないことはコードレビューの機会をすべて奪うと無批判に考える人も多くいます。しかし、このような狭い視点では、数多くの明示的なレビューメカニズムが無視されるだけでなく、おそらく最も強力なコードレビュー手法である、チーム全体による継続的な洗練が軽視されています。
ソフトウェアに関する最も広く行き渡っている考え方の一つは、それが構築して完成させるものであるという考え方です。そのため、建設や建築の比喩が繰り返し使われます。しかし、ソフトウェアの重要な特性はそれが「ソフト」であり、リリース後も初期にプログラマーのエディタで作成された時と同じくらい簡単に修正できることです。そのため、Erik Dörnenburgは賢明にも、アーキテクチャは適切な比喩ではなく、都市計画に置き換える方が良いと主張しています。価値のあるソフトウェアは、それがもたらす価値についての理解が深まるにつれて機能が追加されるため、通常は絶えず変化しています。しかし、機会は新しい機能を追加することだけではありません。そのソフトウェアを洗練し、チームがそのソフトウェアがこれらの変更を可能にする最善の方法について着実に学んだ教訓を取り入れることも重要です。
適切な環境があれば、6ヶ月前に書いたコードの一部を見て、書き方に問題があることに気づき、すぐに修正できます。これは、コードが書かれた時点で欠陥があったか、それ以降のコードベースの変更により、コードが正確ではなくなったためかもしれません。どちらの原因であっても、重要なのは、問題が邪魔になり始めたらすぐに修正することです。コードについて、読んですぐに分からなかったことが理解できたら、(Ward Cunninghamが素晴らしい言葉で言ったように)その理解を頭の中からコードに書き出す責任があります。そうすれば、次の読者はそれほど苦労しなくて済みます。
この洗練のプロセスは、コードレビューで起こることと全く同じですが、コードがコードベースに追加されるときではなく、コードが参照されるたびにトリガーされます。これは私にとって重要な洞察でした。結局のところ、コードレビューが修正しようとする多くの問題は、将来コードが読まれたときに初めて問題になる問題です。それまでは心配する必要がないという強い主張があります。結局のところ、大きなマンションを建設すると交通パターンが変化するように、6ヶ月後にはコードのコンテキストが変化し、コードに必要な修正の種類が変化している可能性があります。また、より多くの人が関与し、このスキームではコードを読むすべての開発者がレビューアーであり、一般的な(しかし、多くの場合曖昧に正当化された)ガイドラインではなく、コードの実際の使用に基づいてレビューできるレビューアーとなります。
あるプラクティスの妥当性を考える一つの方法は、それが独占状態になった場合に何が起こるかを考えることです。唯一のコードレビューメカニズムが後続のプログラマーによる反復であったとしたらどうなるでしょうか?その結果の一つとして、レビューの注意はより頻繁に読まれるコードの領域に集中することになります。これは、主に注意を払うべき領域です。懸念事項としては、決して読まれないコードはレビューされない可能性があることですが、ほとんどの場合、それで問題ありません。優れたテストプラクティスを持つチームは、コードが機能することを確信できます。性能テストは性能上の問題を特定できます。つまり、コードを二度と見る必要がない場合は、それを理解しやすくするために努力する必要はありません。そのようなケースは非常に稀であると予想されますが、これは有益な思考実験です。
しかし、most(ほとんど)≠ all(全て)です。明らかな例外の一つはセキュリティの問題です。攻撃者が脆弱性を見つけるまで何年も正常に機能する可能性がありますが、その時点でレビューの不足を後悔することになります。これは、特別な精査に値する、影響が大きく、稀な安全上の懸念の一例です。しかし、だからといって、洗練をコードレビューメカニズムとして意識的に使用すべきではないという意味ではありません。代わりに、稀で影響の大きい懸念事項を認識し、状況に応じてその種の特定の問題に注意を払うワークフローを調整する必要があるということです。脅威分析は、追加の注意が必要なモジュールと、それらが直面するリスクの種類を警告する必要があります。セキュリティ上の懸念事項については、ターゲットを絞ったコードレビューをスケジュールすることができます。これらは、特定の種類の問題に焦点を当てているため、より効果的に実行できます。
この継続的なコードの洗練を行うためには、他のプラクティスが必要です。コードを変更する場合、既存の機能が壊れないという確信が必要です。そのため、自己テストコードのようなものが必要です。他の開発者にとって大きなマージの競合を引き起こさないことを知る必要があります。そのため、継続的インテグレーションが必要です。効果的にコードを変更できるように、全員がリファクタリングに長けている必要があります。これは、多くの開発者がコードベースのどの部分でも変更することを期待しているため、集団的(または少なくとも弱い)コード所有権が最適です。しかし、これらのスキルを持つチームであれば、通常の洗練をコードレビュー戦略の重要な部分として使用できます。
少なくとも、洗練をコードレビューにおける役割について、より多くの考察を行うことが重要だと思います。プリインテグレーションレビューのみに焦点を当てることの危険性の1つは、コードベースでの変更の仕方が軽視される可能性があることです。もし、私が完璧なメインラインを持ち、そのメインラインにマージされるすべてのコミットが完璧であることを保証したとしても、6ヶ月後もコードベースが完璧であると確信できますか?私はできないと思います。なぜなら、変更とは、6ヶ月前のコードに関する良い決定が現在では良い決定ではなくなっていることを意味するからです。コードを洗練することで、変化する使用方法に対して古いコードを評価し、その健全性を維持できます。