9章 コードレビュー
9.1 コードレビューのフロー
Gerrit Code Reviewは見ている感じ今でもメンテナンスされてそうだから、今も使ってそうですね どう使うんだろうか
コード自体は債務であるということを、覚えておく(そして受け入れる)ことが重要だ。コー ドは必要な債務かもしれないが、それ自体では、どこかの誰かにとっての将来の単なる保守タスクである。
技術的負債についてはよく語られるけど、コード自体が債務であるという考え方は初めて聞いたし、確かにそうだなと納得した
これ最近社内でめっちゃ言っている。。。コードだけじゃなくて、なんらかのメリットを享受するために自分達がメンテナンスする必要があるもの、管理する必要があるものは全て債務なんだよって感覚。
「コード資産」という言葉もあるので、借入をしたときに複式簿記で資産と負債をセットで記入するイメージに近いかも
資産を取得できる代わりに、取得にかかったお金は利子をつけて返済しなくてはならない
メンテが発生するすべてのコードは技術的負債で、議論されているのは有無ではなく規模の大小
9.2 Googleでのコードレビューはどのように機能しているか
悪魔は細部に宿る
細部には神もいるし悪魔もいるから気をつけないと...と思った
god is in the details(神は細部に宿る)は聞いたことあるけど悪魔は聞いたことないなあと思って原文を見たらdevil is in the detailsでちゃんと悪魔だった。神は細部に宿るをネガティブな事象に対して使いたくなって詩人が使ったものらしい。本の内容と全然関係ないけど。
これらのレビュー 3 種全てを 1 人のレビュアーが処理可能なら、全コードレビューをその類のレ ビュアーに処理させておけばよいのではないか。この質問に対し手短に答えるなら、スケールがそ の答えだ。3 つの役目を分けることで、コードレビューのプロセスに柔軟性が加わる。
コードレビューのプロセスをスケールさせるという考え方がなかったので、自分にとって新しい気づきだった
同時にGoogleレベルの大きな企業ならではの考え方だなと思った
うまく言葉にできないけど権限の集中と分散をうまく設計することで統制を加える方向に倒したり、スケールする方向に倒したりと色々できるのかなと思った
分散オーナーシップ構造というのが後に書かれているが、きっと↑の設計がこれのことなんだろう
コードレビューの目的(種類)を3つにわけて、それぞれごとに役割を作るのではなくて、それぞれをどのように担当してもいいっていうやりかた、めっちゃいいなーっておもう。これは仕事の大部分でそうできる気がするな。ちょっと自分にとってめっちゃ新しいやり方な気がする。分類すると担当をつけがちだし、うまくいかなかったときに担当ごとに再分類してしまうんだけど、そうではなくて目的や仕事の内容は変えずにどのような分担にするといいのかっていうほうを変えるの、めっちゃシステマチックでいいな。
役割ごとに人をつけるのか、一人でやるのかはどうでもよく、プロセスを柔軟に変更できるのが大事
RDD: Responsibility Driven Development
ビジネス的な概念→システム的な概念→物理制約
役割を決めて、それらが満たされるようにプロセスを設計していく
アーキテクチャの議論は使っている言語/FW/SaaSに引っ張られて空中戦になりがち
今回みたいに業務プロセスの設計というレベルの抽象度なら話が揃う
システムの実現の手段はなんでもよくて、できるだけ捨てやすい単位でシステムを構築したい
特定の技術スタックにこだわることで負債化するより前にin/outを整理した上で捨ててしまう
今日の前半に「すべてのコードは負債である」という話をしたように、メンテしなくてよい仕組みに置き換える
置き換えによるサービスレベル低下を許容できるPdMだと仕事がしやすい
一度リリースされたものの保守は当たり前だと思われやすい
9.3 コードレビューの恩恵
しかしコードレビュー は義務で、Google の全ソフトウェアエンジニアが参加必須の、数少ない全社的プロセスの 1 つであ る。
厳格なプロセスや手続きの必要性の見極めがきちんとできているのはすごいなと思った
この見極めがないと、ともするとプロセスや手続きを無くして野放図になってしまいそう
レビュアーは個人の意見を理由として代案を出すべきではない。レビュアーは代案を出せるが、意味の把握を強める(例えば複雑さを減らす)場合か、機能性を向 上させる(例えば効率をより上げる)場合に限る。総じて、より「完璧な」解法についての合意を 待つよりは、コードベースを改善する変更を承認していくことをエンジニアは奨励されている。こ のように重点を置くと、コードレビューがはかどる傾向がある
この感覚?が大きな組織内で共有されているのがすごいなと思った
レビュワーによって完璧な解法を求める派と、コードをまずは改善していく派の2つの派閥がある気がする
これ、コードレビューのメンタリングとしての側面を捨てることになるので、全員にある程度のレベル感が要求されそう
例えばFWの流儀に従わないコードが出てきたとき、設計面の指摘をしないと後々チームが苦しむことになる
そういうコードを書いてくる人にはきちんと指摘をしてなぜ良くないのか説明しないと成長しない
意味の把握を強める/機能性を向上させる、がそれに当たるのではないかという話
Linterを入れる目的は「個人の意見」を排除するため
モノリポに対するコミットとコードレビューを通して帰属意識と自己肯定感がましていくみたいな文脈めっちゃ面白いな。
コードレビューは、ある変更がより広い対象にとって理解可能かどうか試す最初の試練であることが多い。
コードは書かれるより読まれる回数が多くなるため、この観点は生死を分けるほどに重要であり、理解と意味の把握が決定的に重要である。
最近はレビューする側が中心なので、読まれるほうが多いんでわかりやすく書いてほしいっていうのはよく言ってるんですが、なかなか浸透せず…(リーダブルコードとか読んで欲しい)
レビュアーは設計上の決定に関して作者を尊重すべきという話とは別に、コードの意味の把握に関する質問に対応する際には「顧客は常に正しい」という箴言(しんげん)を用いるとしばしば役に立つ。
ここの言語化も参考になるなあと思った
コードレビューは知識移転にぴったりの機会で、ちょうどよいタイミングで行われるとともに行動可能なものである
これかなり実感してます
うちのチームは1つのプロダクトに対して新機能開発するグループと保守開発するグループ(メンバーはローテーション)とに分かれていますが、コードレビューをグループで横断してやることで新機能リリース後に保守開発するグループが混乱することが減りました
実際にコードを読んで動かすことや、設計書を見て動作が正しいか確認するプロセスを何日にも渡って踏むことで、理解を助けています
自分で試行錯誤したことは記憶に残りやすい&睡眠で記憶が整理されるので、長い期間同じものに触れたほうがトータルの時間が短くても記憶に残りやすい
9.4 コードレビューのベストプラクティス
レビュアーとなるエンジニアのほとんどは、変更にコメントしてからコメントへの対処を待たずにLGTMを与えるという行動は、その変更にそのコメントの提案する修正を行いさえすれば、その後あらためてレビューを経なくてもリポジトリーへ提出可能であるとの、エンジニア間での信頼と尊敬から来る暗黙の了解に基づいている。
指摘事項の修正後の確認までやってますね…抜けてたり意図と違う修正が返ってきたりというのもあるので。(かといって1から10まで指示するのも相手を軽視してるような感じもあるので)
レビューの指摘で直した箇所は、チェックが薄い&書いてる本人も雑にこなせてバグりやすいので、修正後の確認は大事だと思います
レビュアーの人数は最小限にとどめよ
できればレビュアー1人で運用したいんだけど、どうしてもスキルレベルに差があったり新しく入ってきた人がレビューに参加するハードルが上がったりするので、2人で運用しています
ダブルチェックかつ2人目は必ずシニアになる仕組みのおかげでほとんどバグを出してないのでうまく回ってはいるが、生産性は下がっている
一方で自信がなくても心理的安全性を持ってレビューができる&シニアのレビューと比較ができるので、ジュニアの成長の場にもなっている
みなさんどうしていますか?
レビュアー人数は2人がベストプラクティス、という報告がある
ただ、2人がベストかはすべての実践があてはまっているわけではないという報告もあるので注意
9.5 コードレビューの類型
Googleでは多くの変更は自動生成である。
未来だ!!!!っておもったのを10年以上やっているんだよな。。。すごい。。。
9.6 結論
9.7 要約
みんなからのコメント
Googleにしかできないとしたら、それはなぜか?
人間じゃなくてもいいようなコードの変更を人間がすることがおおい?
高い品質の仕事をスケールするためにコードレビューをしているという文化をつくりにくい?(第二部の内容が効いている気がする)
レビュー可能な人が数十人のチームで一人しかいないとかはありそう
現場でどこまでできているか?
レビューをシステマチックにおこなうことはできていない気がする。(たとえそれがコードじゃなくても)
これの読みあわせをやりました
自分もやりましたー!そのあとレビュー完了の定義やレビュープロセスをチーム全員でモブワークして作った記憶が
この本があるのになぜ実践する企業はすくないのか?
コードレビューをできるようになるための知識や経験の積み方がわからない企業が多そう?
バディとリーダーみたいな関係をつくるみたいなのがあまりなさそうとか、スケールすることとHRTを原則において知識を増やすとかそういったことがなかなか行われていない気がする。ある程度の権限を持った人じゃないと組織的に実践しにくいっていうのがあるのかも?
第2部の内容ほど再現が難しくはない
正しさと意味の把握をするのはレビュアー1人だけ、指摘への対処を確認せずLGTMなどGoogleのエンジニアでないと難しそうな箇所はあるが、プロセスは真似られそう
自動化もpre-commit hookにLinterの自動修正を混ぜることでやりやすくなった
この通りでなくても、近いレベルでやってるところはありそう
それの乗り越え方はなにか?
EM、CTO、VPoEみたいな人たちと事業責任者みたいなひとたちがエンジニアリング組織のグロースについて戦略的に取り組む場を設定するみたいな。
上記「コードレビューの基準」をみんなで読む
ステップを小さくするとしたらどうできそうか?
VPoEがGoogleのソフトウェアエンジニアリングを理解する?(ハードルが高そう
事業責任者向けのエッセンシャルGoogleのソフトウェアエンジニアリングを説明するとか?(ハードルが高そう
思えば、牛尾さんのRSGT2021のクロージングキーノートはこういう話だったのかもしれない。