読書メモ: Looks Good To Me

こちらの本を読みながら書いた読書メモをちょっと整えて公開します。読書メモの後にわたくしのコードレビューに対する私見を書こうと思います。

https://amzn.to/43XmhWu

コードレビューとは

では、コードレビューとは何でしょうか？その本質は、ソフトウェア開発者が、互いのコードを検査し、合意された一連の基準を満たしているかを確かめるプロセスです。

第1章〜第3章

基礎的なことなので、あんま学ぶことないかも

第4章: TWA

  • Team Working Agreement
  • レビューの期待値についてチーム内で合意を取る
  • ラリーの回数、応答時間、どこまで深くレビューするか、など

そもそもチームにジュニアレベルがいるかどうかに依存しそう。ある程度のレベルの人しか取らない環境であれば、そんなに気にする必要がなさそう

第5章: 自動化

  • LintとかFormatterとかPRテンプレートとかレビューの自動割り当てとか
  • 細々としたことが書いてあって、冗長だなと思った
  • LintとFormatが意味違うの知らなかった
    • Lint: コードの品質チェック
    • Fortmat: コードの機械的なフォーマットチェック
    • 実際重複するチェックは多いけども

第6章: レビューコメント

  • 「xxxに変えた方がいい」と書くときは理由も考える
    • これはやっている
  • 理由を考えて、指摘を見送るor指摘するor延期するを判断
  • 一般的に使われているコメントシグナルの例
  • 他にも紹介されているけど省略
  • 個人攻撃にせず、丁寧なコードの不備の指摘を行う、も当然のことかなあ
    • 当然のことを明文化するのも大事だけど
  • 個人的にレビュー指摘の考え
    • 絶対に変更が必要 / 変更した方が良い / 好みの問題 / 気になった ぐらいのグラデーション
    • ラリーが長くならないようにはしたい
    • ジュニアエンジニアのレビューはまた変わりそう

第7章: コードレビューがダメになる理由

  • アンチパターンが色々紹介されている
  1. いい加減なコードレビュー
    1. 意地悪なコードレビュー
      1. 変幻自在なコードレビュー
        1. 厳しすぎるコードレビュー

        第8章: コードレビューの遅延を減らす

        コードレビューを効果的にするための対策

        • レビュアーをシニアエンジニア一人に依存しないようにする
          • 承認権限をチーム全員に持たせる
          • 全てのPRのレビュアーをシニアに割り当てない
          • よく知らないコードは学習の機会と捉える
        • PRにコンテキストを含めて、レビュアーの理解を助ける
        • PRが大きすぎる
        • 機能が大きすぎる
        • 議論の応酬が多すぎる
        • 先にリファクタリングが必要

        第9章+第10章

        コードレビュープロセスの抜け穴を潰そうという章

        • フィードバック文化の欠如、に対する指摘はあるよなあと思った
        • 議論の中心になっている、「緊急事態だからレビュープロセスを飛ばそう」という事象は身の回りであまり観測できなくて、ピンと来なかった
          • ホントの緊急対応の汚いhotfix出した後に時間できたら整える、はやるもんなあ

        第11章〜13章

        • それぞれペアプロ・モブプロ・AI活用の章
        • ペアプロもモブプロもやってる会社入ったことない
        • 人がコード書いてるとこ見たいとも、見て欲しいとも思わないんだよなあ

        本の感想

        ちょっと期待値が高すぎて、微妙だと思ってしまった。もうちょっと実践的なプラクティスを期待していたけど、教科書的なことが書かれていた印象。「これを導入したけど上手くいかなくてこう調整したら上手くいった」のような話が欲しかった。

        Team Working Agreementと緊急時対応プレイブックをつくろう、は実際やっても形骸化しそうな気がするんだよなあ……

        個人的コードレビューの課題感

        正直一定レベルのコード書ける人で固めたチームだと、コードレビューで指摘することはそんなになくて、むしろ有意義な指摘がどうしたらできるだろうと考えることが多かったように思う。

        個人的に意識してること

        • コードレビューはあくまでコードレビューで、バグの発見はQAフェーズ
        • 否定的なことを書くときは慎重かつ丁寧に書く
        • 修正を提案するときは絶対に理由を書く
        • 基本その日にレビュー、遅くても翌日
          • マージまでのスピードは開発者体験に大きく関わる
        • 好みの問題は指摘しない
        • ラリーが長くなりそうなときはSlackに移行する
          • ラリーは1PRに対して20件以内ぐらいに留めたい

        この辺は開発チームにジュニアレベルの人がいると、この限りではないはず。コードレビューに教育の意図も入ってくると思うし。

        Team Working Agreementみたいな明文化された文章がそんなに上手くいかない気がするのは、たぶん完璧な合意ドキュメントをつくるのが非常に困難で、つくれたとしてもそれを守るのが困難というところだろう。

        実際はチームの文化でプロセスをよくして、極力ルールは少なくしたい。そしてルールの前に、PRのテンプレート機能やLint/Formatterの設定、Dangerによるワーニングで対処できないかを考える。それでも問題が出るようならルール。

        カルチャー > 自動化 > ルール なのかな。チームの開発レベルが落ちれば落ちるほど、多くのルールが必要となり、開発は非効率化して、悪循環に陥るので、基本的には開発レベルを落とさないというのがコードレビュープロセスにも効いてくるように思う。

        で、開発レベルが一定水準超えてる前提で、自分がコードレビューするとなったときに、何を指摘すると有意義で、何を指摘すべきでないかで課題感がある。

        • 明らかなミスの指摘
        • 難読化している箇所の指摘
        • 設計や命名の良くない箇所の指摘
        • 意図のわからない処理についての質問

        実際やってること（やれてないときもあるけど）はこんなところか。

        （了）