コードレビューに関する資料メモ
一般に、CL が完璧でなくても、その変更がシステムのコードの全体的な健康状態を改善すると確実にわかれば、レビュアーは CL を積極的に承認すべきである。
これはコードレビューの全ガイドラインで最上位の原則です。
CL: 「changelist」の略。一つの独立した変更を意味していて、バージョン管理に提出されたものや、まだコードレビュー中のものを指すこともあります。他組織では「変更」「パッチ」とよく呼ばれます。
「完璧な」コードといったものは存在しないということです—存在するのはより良いコードだけです。
知識の共有は、時間をかけてシステムのコードの健康状態を改善する試みの一部分です
コードレビューで意見の対立があれば、最初のステップで必ず行うべきは、このドキュメントとCL 作成者のガイド、またこのレビュアーガイドの他のドキュメントに基づいて、開発者とレビュアーの間でコンセンサスが得られるように調整することです。
作成者とレビュアーが合意に達することができないからといって、CL をそのまま放置しないでください。
「複雑すぎる」とは普通、「コードを読んですぐに理解できない」という意味です。 あるいは、「開発者がこのコードを呼び出したり修正したりしようとするときに不具合を生み出す可能性がある」という意味でもあります。
レビュアーはオーバーエンジニアリングを特に警戒すべきです。 開発者には、現在解決する必要のある既知の問題に取り組むべきであって、将来解決する必要が出てくるかもしれない推測に基づいた問題には目を向けないよう勧めてください。
テストの有効性は人間が確認しなければなりません。
コードの健康状態を悪化させる CL を受け入れないでください。 ほとんどのシステムは小さな変更が積み重なってだんだんと複雑化します。 だからこそ、新たな変更があったときに小さな複雑性でも混入させないようにするのが大切です。