コードレビューと私の過去と未来

by jx.matsushita

Slide 1

Slide 1 text

コードレビューと私の過去と未来松下純@XP祭り2024 2024.9.28

Slide 2

Slide 2 text

2 自己紹介松下純 @jxmtst.bsky.social フリーランスのテックリードコミュニティ運営 ScrumMastersNight! ScrumDevelopersNight! 支援先 #サッカー #旅行 #スプラ #自キ

Slide 3

Slide 3 text

3 コードレビューで疲弊したこともあったけど気がついたら上手いことレビューできる様になってきた今日話すこと

Slide 4

Slide 4 text

4 何に疲弊していて何をしたら今どんな感じなのか今日話すこと

Slide 5

Slide 5 text

5 2016年から運用されているプロジェクトのメインモノリス初めてのプルリクエストのレビューは2017年2月10日現在までにコメントをつけたプルリクエストは1,697件 (マイクロサービスのコメントしたPRは合計1,300件超)

Slide 6

Slide 6 text

6 昨年から参画したプロジェクト初めてのプルリクエストのレビューは2023年10月24日現在までにコメントをつけたプルリクエストは296件途中別プロジェクトで稼働中にコメントしたPRは21件

Slide 7

Slide 7 text

7 稼働日が年間240日だとして平均して一日約 2.8件のプルリクエストに目を通してコメントしている *LGTMだけのコメントも含まれる

Slide 8

Slide 8 text

8 実装もマネジメントもしているけどそんなにレビューをして疲弊しないの？

Slide 9

Slide 9 text

9 疲弊していた時期もありました

Slide 10

Slide 10 text

10 レビュー期間：2018.6.1 – 6.26 (26days) コメント数：266コメントファイル変更差分：281ファイル（大量のサンプル画像を含むプログラムは98ファイル) 納得感のないコメントで〆本人の見積もり

Slide 11

Slide 11 text

11 レビュー期間：2018.8.3 – 9.18 (46days) コメント数：218コメントファイル変更差分：33ファイル Approveコメントなしで別ブランチにマージ、さらに実装が続く

Slide 12

Slide 12 text

12 どうしてこうなったのか彼は隣の席に座っていたのに！

Slide 13

Slide 13 text

13 どうしてこうなったのか画面イメージのみで共有された設計がなかった引数や返り値の型が書かれていなかったちょくちょく混じるTypo（farst→first など）単数系と複数形の区別が適当抽象度がバラバラ使用していないファイルをコミットするインデントがバラバラ CIがなかったので自動テストが通っていない状態でレビュー依頼をされていたレビュワーの指摘の全てをレビュイーが修正していた自動テストがない実装変数名が適当（itemsとitem_listが同じスコープ内に存在した）時間をかけてプライベートメソッドのテストを書いていた実装の範囲が広い（フロントエンドの実装とバックエンドの実装が含まれていた）

Slide 14

Slide 14 text

14 最近はどうですか？

Slide 15

Slide 15 text

15 最近はどうですか？平均レビュー時間：12分/件平均コメント数：5.8件 85件のプルリクエストが当日中にApproveされている直近100件のプルリクエスト

Slide 16

Slide 16 text

16 何が変わったのか

Slide 17

Slide 17 text

マスタータイトルの書式設定そもそもコードレビューとは

Slide 18

Slide 18 text

マスタータイトルの書式設定コードレビューとはコードレビューは、コードが作者以外の者によってレビューされるプロセスで、多くの場合そのコードがコードベースに入る前に行われる。というのがコードレビューの簡単な定義だが、コードレビュープロセスの実施形態については、ソフトウェア業界全域の各所で大いに異なる。 Googleのソフトウェアエンジニアリング (p.195) 2021.11

Slide 19

Slide 19 text

マスタータイトルの書式設定コードレビューの根本コードレビューが答えるべきいちばん根本的な質問は「自分はこれのメンテナンスを問題なく行えるか？」です。脳に収まるコードの書き方 (p.153)

Slide 20

Slide 20 text

マスタータイトルの書式設定コードベースは誰のものでしょうか？

Slide 21

Slide 21 text

マスタータイトルの書式設定チームによるコードの共同所有ＸＰでは全員がシステム全体に対して責任を持つ。すべてのコードを知っているわけではないが、何らかの知識はある。誰でもいつでもシステムのすべてのコードを変更できる Collective Ownership

Slide 22

Slide 22 text

マスタータイトルの書式設定コードレビューはレビュワーとレビュイーの二人だけではなくチームのためのプラクティス

Slide 23

Slide 23 text

23 どうしてこうなったのか彼は隣の席に座っていたのに！

Slide 24

Slide 24 text

24 どうしてこうなったのかチームが共同所有すべきコードベースをチームの誰が見ても触れる様にしようという意識がレビュワーとレビュイー両方に欠けていた。コードレビューのゴールがわからなかったので迷走した。

Slide 25

Slide 25 text

マスタータイトルの書式設定レビュイーとレビュワーの二人だけではなく誰でもいつでも提出されたコードを変更できる

Slide 26

Slide 26 text

マスタータイトルの書式設定コードレビューを楽にするプラクティス

Slide 27

Slide 27 text

マスタータイトルの書式設定コーディング標準 coding-standard コードレビューを楽にするプラクティス特に変数/関数の命名で揉めがちユビキタス言語まではいかなくともチームの辞書の様なものがあると良いチームのデザインパターンを整備して積極的に使ってもらうコーディング標準に固執すると逆にコードの保守性を損なうこともあるので定期的な見直しを行なった方がよい

Slide 28

Slide 28 text

マスタータイトルの書式設定自動化する automation コードレビューを楽にするプラクティス言語のスタイルガイドはフォーマッターやリンターを利用して自動でかけておくようにするとレビュワーはレビューの観点から書式整形を気にしなくて良くなる。チームのデザインパターンがある場合は雛形を作るジェネレータを作成しておくとよりドメインロジックにフォーカスしたレビューがしやすくなる。継続的インテグレーションでユニットテストやリグレッションテストを行う。ただし、時間がかかりすぎるCIはフィードバックループのパフォーマンス低下につながるので、テストの実行時間については気をつけた方が良い。

Slide 29

Slide 29 text

マスタータイトルの書式設定変更はちょうど良いサイズに comfortably sized change コードレビューを楽にするプラクティス大きなコードの変更は手戻りの危険もあるばかりか、レビュワーの欠陥発見能力の低下につながってしまう。タスクが小さすぎて実装をするよりレビュー依頼のドキュメントを書く時間の方がかかる様なことは本末転倒だが、数時間から長くても一日で実装が終わるくらいの大きさに設計を分割する。変更がちょうど良いサイズになる様にするとレビューが楽になる。実装の途中で設計に不備が見つかり変更のサイズが大きくなりそうな場合は相談して分割できないかを検討する。

Slide 30

Slide 30 text

Slide 31

Slide 31 text

マスタータイトルの書式設定テストコードは書く/書かせる should write test コードレビューを楽にするプラクティス「急いでいるのでテストコードは後で書きたい」というレビュー依頼を受けることがある。しかし、結局テストコードがないことによる手戻り、バグの混入、手動での確認に時間が取られて結果として急ぐどころか遅くなる。テストコードを書く時間がないのではなく、単純なスキル不足なことがほとんどなので、実装にかかるコストとして受け入れた方が良い

Slide 32

Slide 32 text

マスタータイトルの書式設定本当に必要なことだけ only essentials コードレビューを楽にするプラクティスレビューをするコードの変更が多くなればなるほど、レビュワーの負担は認知負荷は増え、リードタイムが伸びてしまいます。ついでにここも直したいという気持ちが湧くこともあるかもしれませんが、別のブランチを切って別のレビュー依頼として出し直す様にすると、お互いが幸せになれます。

Slide 33

Slide 33 text

マスタータイトルの書式設定デイリーレビュー daily review コードレビューを楽にするプラクティス変更がちょうど良いサイズになる様に設計しても、日をまたいで実装に時間がかかってしまうことがある。途中でも良いのでコードの実装状態を共有しておくと中間レビューの様な役割を果たせるので、問題となりそうな箇所を早めに見つけることができる。あくまでも実装の途中なので細かく見過ぎない方が良い。

Slide 34

Slide 34 text

マスタータイトルの書式設定レビュワーはデバッガーではない reviewer, not debugger コードレビューを楽にするプラクティスコードレビューはコードの品質を上げて欠陥を減らす側面があることは間違いないが、バグを見つけることが主目的ではない。要求どおりの価値を提供できていて他の価値を損なわないことが満たされていれば、軽微なバグは後で直しても良い

Slide 35

Slide 35 text

マスタータイトルの書式設定レビュイーを尊重する respect reviwee コードレビューを楽にするプラクティスコードレビューはチェンジセットを拒否することが可能だが、それはコードレビューの関係性においてレビュワーがレビュイーよりも偉いことを意味しない。（たとえ職位が上だとしても）要求を満たすために複数の実装の選択肢が存在していても、ガイドラインから外れていないのであれば、レビュワーの嗜好よりもレビュイーの選択を尊重する。

Slide 36

Slide 36 text

マスタータイトルの書式設定コードレビューの類型 • グリーンフィールドレビュー • 完全に新しいコードのレビュー • 新しいコードまたはプロジェクトは、コードレビューとは別に相当な量の設計レビューを経ることが求められる。コードレビューは、過去に既に行われた設計上の決定について討議するための時間ではない • 挙動の変更、改善、最適化 • バグ修正とロールバック • リファクタリングと大規模変更 • 機械の生成した変更であってもレビューを要する Googleのソフトウェアエンジニアリング (p.213-)

Slide 37

Slide 37 text

マスタータイトルの書式設定コードレビューの類型によってレビュー形態を変えるグリーンフィールドレビューの様な新しい実装のレビューは、完成をしていなくても、例えばペアプログラミングやソフトウェアチーミング(モブプログラミング)の様な継続的なコードレビューを行うことで、早期の相談などができる。一方で対面でのレビューはレビュイーに対する忖度や場のフォースが働くため、レビュワーの欠陥発見能力が低くなる。バグを発見するためには、自分のペースでコードを眺める時間を作るべき。

Slide 38

Slide 38 text

マスタータイトルの書式設定今後コードレビューで試してみたいこと

Slide 39

Slide 39 text

マスタータイトルの書式設定 LLMによるコードレビューの補佐今後コードレビューで試してみたいこと LibSassをDartSassに変更する実装をした際に出力結果をLLMに比較してもらったところ、目でdiffを追うだけではわからなかった事も見つけてくれた。変更があったドメインロジックだけを抜き出すなど、使い道はいろいろありそうなので、LLM利用を自動でかけることでコードレビューが楽にならないか考えてみたい。

Slide 40

Slide 40 text

マスタータイトルの書式設定コードレビューのKPIにこだわる今後コードレビューで試してみたいこと今回の登壇をするにあたって、リードタイムの計測などを試みてみたが上手くとれなかったので、メトリクスを計測していく仕組みを作りたい。コードレビューのリードタイムはまだまだよくなりそうな肌感もあるので、改善もあわせて行なっていきたい

Slide 41

Slide 41 text

マスタータイトルの書式設定一番大事なのはビジネス価値を産み出すことすばやくリリースをしてフィードバックサイクルを機能させることボトルネックになりがちな非同期コードレビュー

Slide 42

Slide 42 text

マスタータイトルの書式設定コミュニケーションシンプリシティフィードバック勇気リスペクトエクストリームプログラミングの5つの価値

Slide 43

Slide 43 text

マスタータイトルの書式設定 References Googleのソフトウェアエンジニアリング / Titus Winters, Tom Manshreck, Hyrum Wright 編 / 竹辺靖昭監訳 / 久富木隆一訳 / O’REILLY 2021.11 脳に収まるコードの書き方 – 複雑さを避け持続可能にするための経験則とテクニック / Mark Seeman 著 / 吉羽龍太郎, 原田騎郎訳 / O’REILLY 2024.06 Making Software / Andy Oram, Greg Wilson / O’REILLY 2011.09 エクストリームプログラミング / Kent Beck, Cynthia Andres 著 / 角征典訳 / オーム社 2015.06 iki-iki - https://scrapbox.io/iki-iki/