Googleのソフトウェアエンジニアリングから学ぶコードレビュー
オライリー・ジャパンから「Googleのソフトウェアエンジニアリング」という翻訳の書籍が発売され、その中からコードレビューに関する箇所を読んで学んだことの紹介です。
サブタイトルの持続可能という表現が良い
この本のサブタイトルは「持続可能なプログラミングを支える技術、文化、プロセス」です。
この持続可能(sustainable)の部分がとても響くものがあって、持続可能とするためにはどうしたら良いだろうか? これを根源的な欲求として持ち、コードとコードレビューに向き合うのが本書では語られています。
実は、原著のサブタイトルは「Lessons Learned from Programming Over Time」で、
持続可能という直接的な表現はありません。 Over Time が刻を超越して、転じて持続していく様子につながると感じます。
持続可能の表現は、本文序文に sustainable として登場します。
What practices can we introduce to our code to make it sustainable—able to react to necessary change—over its life cycle, from conception to introduction to maintenance to deprecation?
日本語訳は次の通りです。
自分たちのコードを、着想し、導入し、保守し、廃止するまでのライフサイクルを通じて持続可能(sustainable)なものとするためにコードに導入できるのは、どんなプラクティスだろう?
はじめに
発売当初に購入して、コードレビューの章にかなり影響を受け、この1年近く、取り込めそうなものは意識して取り込み、コードレビューを行なってきました。
本書から特に印象に残ったキーワードを紹介します。
自分にとって引き出しやすい名前をつけるのは、ことあるごとに思い出しやすくし、道具として使いやすくします。本書の章立てとは完全には一致していません。
- 理解と意味の把握
- 質問する
- 迅速に
- 断片的な返信は避ける
- コードはチームのもの
- コードは債務
- 変更を小さく
- 1行にまとめる
- リーダビリティ
以下、本書を引用しながら自分のコンテキストで色付けしてより深く根付かせるようにしていきます。
理解と意味の把握
第9章がコードレビューに費やされています。
その中で一番印象に残ったのはコードレビューの恩恵について書かれた次の一節です。
通常はコードレビューが、作者以外の者がコード変更を検証する初めての機会である。こうした観点によってレビュアーには、最も優れたエンジニアですらできないことができるようになる。それは、コード作者の観点から生じるバイアスに影響されないフィードバックを提供することだ。コードレビューは、ある変更がより広い対象にとって理解可能かどうか試す最初の試練であることが多い。コードは書かれるより読まれる回数が多くなるため、この観点は生死を分けるほどに重要であり、理解と意味の把握が決定的に重要である。
この章では繰り返し、理解と意味の把握といった点について表現を変えて伝えているように感じました。
コード作者の手から離れて、別の誰かがメンテナンス可能な状態になっているか、それを確認する作業がコードレビューという位置付けです。
Googleにおいてもコードレビューで得られる恩恵の一つとしてコードの正しさを挙げており、本書を読む前はこれが最重要だと考えていました。
しかし、コードレビューのプロセスにおいてコードの正しさを担保するのは難しく、実際に細心の注意をしても見逃すことはあり得ます。そのため、ここが完璧であることを求めません。
コードレビューは、コードの正しさについての万能の解決策でも唯一のチェック方法でもなく、ソフトウェアをめぐるそのような問題に対抗する多層防御の一要素である。結果として、コードレビューが成果を上げるためには「完璧」である必要はない。
実に意外なことに、コードの正しさのチェックは、コードレビューのプロセスからGoogleが得る恩恵の首位ではない。コードの正しさのチェックは一般的に、コード変更が動作することを保証する。だが、時間が経ちコードベース自体がスケールした場合もコードの変更が理解可能で意味を成すと保証することの方が、意義が大きい。
質問する
コード作者を尊重し、レビュアーは質問することを推奨しています。
前述の通り、根底にあるのは、理解と意味の把握です。そのために質問を行うアプローチが導き出されています。
コードレビュープロセスにおける、フィードバックと確認を行う段階では、変更がその特定の方法で行われているのは何故かという質問を投げかける必要がある。この情報交換が、知識共有を促進するのだ。実際、多くのコードレビューには双方向の情報交換が伴う。作者もレビュワーも、レビューツール自体の中で作者に編集提案を直接共有しても差し支えない。
一般に、レビュアーはコード作者の特定アプローチについて作者を尊重すべきであり、作者のアプローチが不十分な場合にのみ代案の指摘を行うべきだ。作者が同等の妥当なアプローチをいくつか示せるなら、レビュアーはその作者の選好を受け入れるべきである。
そのアプローチが間違っていると決めてかかる前に、何故そのようなアプローチが採られたかについて質問した方がよい。
迅速に
コードレビューは、迅速に行われるべきであることについても言及されています。
24(勤務)時間以内なので、1日8時間ペースなら3営業日は妥当なラインだと思います。場合によってはこれでも遅く、早ければ早いほど良いでしょう。
コード作者からしても何日か前に作成したコードのフィードバックをもらうより、さっき作成したコードのフィードバックをもらった方が対応が楽です。
レビュアーは、迅速にフィードバックを行うべきである。Googleでは、24(勤務)時間以内にコードレビューからのフィードバックが行われることが期待される。レビュアーがその時間内にレビューを完了できないなら、変更を少なくとも目にしてはいてできるだけ早くレビューに着手予定であると返信するのが、良い(そして期待される)プラクティスである。
断片的な返信を避ける
コードレビューでは、断片的な返信を避けることも重要な要素としています。
まとまったフィードバックの対応の方がコード作者がまとめて対応できるので楽です。
レビュアーは、コードレビューに少しずつ断片的に返信することも避けるべきだ。レビュープロセスでは、レビューでフィードバックを得て対処してから無関係なフィードバックをさらに受け続けることほど、コード作者にとって苛立たしいことも滅多にない
レビュワーは、これはコードレビューに限らず、アプリやデザインレビューにおいても、思いついたフィードバックを五月雨に返す方は楽ですが、フィードバックを受け取る方は苛立つだけならまだ良い方で、精神的に疲弊するケースもあり、特に注意したいです。
もし断片的な返信になってしまいそうなコードの状態である場合、そうなってしまいそうな予告と共に根本的な対応が必要な状況であることが想定されるため、非同期のレビューよりも同期的なレビューの実施が有効です。
コードはチームのもの
レビュアーがコード作者を尊重する前提として、コード作者側に、作成時点の責務を担うことを期待しています。提案されたコードはレビュアーと共有することで作者の責務ではなく、チームの責務になります。チームの責務にして良いものなのか、その判断においてはコード作者の責務として期待しているということになります。
自分が提案するコードは「自分のもの」ではなくチームのものであることを思い出してほしい。コードベースにそのコードをチェックインした後は、いかなる場合でもそのコードはもはや自分のものではない。自分のアプローチについての質問を歓迎し、自分がある特定の方法で物事を処理した理由を説明する心構えをしておくべきだ。作者が担う責務が、将来にわたりそのコードが理解可能で保守性があるよう保証するという要素を含むことを、肝に銘じておかなければならない。
コードは債務
コードは債務である、このことを根底に意識することでコードに対する接し方も変わってきます。様々なレビュー観点があると思いますが、常に、変更する理由に対して追加する債務は許容範囲内かどうかを見ていくのがコードの品質を保つ上で重要になってくると思います。
コード自体は債務であるということを、覚えておく(そして受け入れる)ことが重要だ。
新機能は必要な場合が多いのは当たり前だ。だがそもそもコードを開発する前に、どんな新機能についても開発に足る正当な理由があることを確かめるよう気をつけておくべきである。
完全に新しいコードは非常に顰蹙を買うので、我々の間では「ゼロから起きこしているなら、間違っている」という言い習わしまである。
このことは、ライブラリーやユーティリティのコードには特に該当する。おそらくは、ユーティリティを書いている場合、Googleほどの規模のコードべーす内なら、他の誰かがどこかで似たようなことを多分やってしまっているだろう。
Googleに限らず、複数のコードを組織として扱っているなら、横断的にコードを探索できる仕組みも非常に重要であり、テックリードを置いている組織においては、この部分の仕組み化を推進していくのが期待される役割であろうと感じました。
したがって、17章で論じられる類のツールは、そのようなユーティリティコードを見つけ出す目的と重複したコードの導入を防ぐ目的の両方のために、決定的に重要なのだ。
変更は小さく
これらを実現するために変更を小さくするというのが非常に重要なプラクティスです。
コードレビューのプロセスを機敏なものに保つ上で最も重要なプラクティスはおそらく、変更を小さくとどめておくことだ。理想的にはコードレビューは、レビュアーと作者双方のために、噛み砕いて理解することが容易で、かつただ1つの問題に集中したものであるべきだ。
1行にまとめる
本書では、1行目という表現を重要視しています。GitHubのPull Requestのタイトルが1行目に相当すると思って良いと思います。
変更説明というものは、1行目に要約としてその変更がどんな種類のものかを示すべきだ。1行目は、一等地の不動産のように貴重な位置を占める。1行目は、コードレビューツール自体の中での要約を提供する
これはGitのコミットメッセージにも同様のことが言えます。小さく、明快な変更を行なっていれば、タイトルは自然と決まります。これを意識するだけで、もしタイトルに何を書いたら良いのか迷うことがあった場合に、その変更は複雑になっている兆候と捉えることもできると考えます。
何が変更されているのか、そして何故変更されるのかの詳細についても説明が及ぶべきである。「バグ修正」という説明があるだけでは、レビュアーや、将来のコード考古学者によっては助けにならない。
ここでも理解について繰り返し強調しています。正しさを基準とはせず、他者が、特に未来の自分たちが後になっても理解しやすいものになっているかを基準とし、そのために変更説明を含めてレビュー対象になっています。
変更内で自分がやったことの理由をレビュアーが理解できないなら、自分がやったことがたとえ正しいとしても、そのようなコードの構造かコメント(またはその両方)は改善が必要だということがはっきりと表されている。
コードレビューは、現在自分が行う活動であるだけでなく、自分の行ったことを後世に向けて記録するために行う活動なのだ。
リーダビリティ
ここまでコードレビューの章から引用でしたが、Googleのコードレビューの大前提としてリーダビリティという考えがあります。これは第3章の知識共有で述べられています。
順番としても第3章、第9章という流れなので、このリーダビリティをベースにしてコードレビューがあるのですが、一旦コードレビューに対する考え方としてまずは吸収することを優先しました。実務としてコードレビューを担当することが多いので、実際に第9章から読み始めました。(第1章が難解で挫折しそうになったので最初から通しで読むことを諦めました)
Googleでは、「リーダビリティ(readability)」は単なるコードの読みやすさ以上のことを指している。リーダビリティは、プログラミング言語のベストプラクティスを普及させるための、Google全社的な標準化されたメンター制度のプロセスである。プログラミング言語のイディオム(idiom)、コード構造、API設計、共通ライブラリーの適切な利用、ドキュメンテーション、テストのカバレッジ(coverage:対象範囲)などに加え、その他各種の広範囲な専門知識をリーダビリティは扱っている。
Googleの初期、Craig Silverstein(従業員ID3番)は自ら、新入社員全員の各々と一緒に席に着いて、その新入社員が初めて行う大きなコードのコミットに対し1行ずつ「リーダビリティレビュー」を行なったものだった。そのレビューは重箱の隅をつつくようなもので、コードの改善可能な点から空白文字の慣例まで全てを対象としていた。このレビューにより、Googleのコードベースが均一な体裁を保つようになっていた。だがもっと重要なのは、このレビューがベストプラクティスを教え、どのような共有インフラストラクチャーが利用可能かを際立たせ、新入社員にGoogleでコードを書くとはどういうことかを示していたということだ。
今日では、任意の時点において、Googleのエンジニアのうち約20%がレビュアーもしくはコード作者としてリーダビリティプロセスに参加している。
このリーダビリティを踏まえた、リーダビリティレビューもコードレビュープロセスにおいてGoogleでは行なっています。今まで上げてきたコードレビューの指針はリーダビリティを前提とした上で成立する側面もあります。
まとめ
リーダビリティを今も実現できる自己評価にはありません。とはいえ、本書のコードレビューの指針は自分にとって良い方針だと感じており、リーダビリティについては継続して学びつつ、本書のコードレビューの指針を取り込んで、引き続きコードレビューを実践していこうと思っています。
Discussion