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

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/

Transcript

1. コードレビューと私の過去と未来 松下 純@XP祭り2024 2024.9.28
2. 2 自己紹介 松下 純 @jxmtst.bsky.social フリーランスのテックリード コミュニティ運営 ScrumMastersNight! ScrumDevelopersNight! 支援先 #サッカー #旅行 #スプラ #自キ
3. 3 コードレビューで疲弊したこともあったけど 気がついたら上手いことレビューできる様に なってきた 今日話すこと
4. 4 何に疲弊していて何をしたら今どんな感じなのか 今日話すこと
5. 5 2016年から運用されているプロジェクトのメインモノリス 初めてのプルリクエストのレビューは2017年2月10日 現在までにコメントをつけたプルリクエストは1,697件 (マイクロサービスのコメントしたPRは合計1,300件超)
6. 6 昨年から参画したプロジェクト 初めてのプルリクエストのレビューは2023年10月24日 現在までにコメントをつけたプルリクエストは296件 途中別プロジェクトで稼働中にコメントしたPRは21件
7. 7 稼働日が年間240日だとして 平均して一日約 2.8件のプルリクエストに目を通してコメントしている *LGTMだけのコメントも含まれる
8. 8 実装もマネジメントもしているけど そんなにレビューをして 疲弊しないの？
9. 9 疲弊していた時期もありました
10. 10 レビュー期間 ：2018.6.1 – 6.26 (26days) コメント数 ：266コメント ファイル変更差分 ：281ファイル（大量のサンプル画像を含む プログラムは98ファイル) 納得感のないコメントで〆 本人の見積もり
11. 11 レビュー期間 ：2018.8.3 – 9.18 (46days) コメント数 ：218コメント ファイル変更差分 ：33ファイル Approveコメントなしで別ブランチにマージ、さらに実装が続く
12. 12 どうしてこうなったのか 彼は隣の席に座っていたのに！
13. 13 どうしてこうなったのか 画面イメージのみで共有された設計がなかった 引数や返り値の型が書かれていなかった ちょくちょく混じるTypo（farst→first など） 単数系と複数形の区別が適当 抽象度がバラバラ 使用していないファイルをコミットする インデントがバラバラ CIがなかったので自動テストが通っていない状態でレビュー依頼をされていた レビュワーの指摘の全てをレビュイーが修正していた 自動テストがない実装 変数名が適当（itemsとitem_listが同じスコープ内に存在した） 時間をかけてプライベートメソッドのテストを書いていた 実装の範囲が広い（フロントエンドの実装とバックエンドの実装が含まれていた）
14. 14 最近はどうですか？
15. 15 最近はどうですか？ 平均レビュー時間：12分/件 平均コメント数 ：5.8件 85件のプルリクエストが当日中にApproveされている 直近100件のプルリクエスト
16. 16 何が変わったのか
17. マスター タイトルの書式設定 そもそも コードレビュー とは
18. マスター タイトルの書式設定 コードレビュー とは コードレビューは、コードが作者以外の者によってレビューされるプロセス で、多くの場合そのコードがコードベースに入る前に行われる。というのが コードレビューの簡単な定義だが、コードレビュープロセスの実施形態につ いては、ソフトウェア業界全域の各所で大いに異なる。 Googleのソフトウェアエンジニアリング (p.195) 2021.11
19. マスター タイトルの書式設定 コードレビュー の根本 コードレビューが答えるべきいちばん根本的な質問は 「自分はこれのメンテナンスを問題なく行えるか？」 です。 脳に収まるコードの書き方 (p.153)
20. マスター タイトルの書式設定 コードベース は 誰のものでしょうか？
21. マスター タイトルの書式設定 チームによるコードの 共同所有 ＸＰでは全員がシステム全体に対して責任を持つ。すべての コードを知っているわけではないが、何らかの知識はある。 誰でもいつでもシステムのすべてのコードを変更できる Collective Ownership
22. マスター タイトルの書式設定 コードレビューは レビュワーとレビュイーの二人だけではなく チームのため のプラクティス
23. 23 どうしてこうなったのか 彼は隣の席に座っていたのに！
24. 24 どうしてこうなったのか チームが共同所有すべきコードベースを チームの誰が見ても触れる様にしようという意識が レビュワーとレビュイー両方に欠けていた。 コードレビューのゴール がわからなかったので迷走した。
25. マスター タイトルの書式設定 レビュイーとレビュワーの二人だけではなく 誰でもいつでも提出されたコードを変更できる
26. マスター タイトルの書式設定 コードレビューを楽にするプラクティス
27. マスター タイトルの書式設定 コーディング標準 coding-standard コードレビューを楽にするプラクティス 特に変数/関数の命名で揉めがち ユビキタス言語まではいかなくともチームの辞書の様なものがあると良い チームのデザインパターンを整備して積極的に使ってもらう コーディング標準に固執すると逆にコードの保守性を損なうこともあるので 定期的な見直しを行なった方がよい
28. マスター タイトルの書式設定 自動化する automation コードレビューを楽にするプラクティス 言語のスタイルガイドはフォーマッターやリンターを利用して自動でかけておく ようにするとレビュワーはレビューの観点から書式整形を気にしなくて良くな る。 チームのデザインパターンがある場合は雛形を作るジェネレータを作成しておく とよりドメインロジックにフォーカスしたレビューがしやすくなる。 継続的インテグレーションでユニットテストやリグレッションテストを行う。た だし、時間がかかりすぎるCIはフィードバックループのパフォーマンス低下につ ながるので、テストの実行時間については気をつけた方が良い。
29. マスター タイトルの書式設定 変更はちょうど良いサイズに comfortably sized change コードレビューを楽にするプラクティス 大きなコードの変更は手戻りの危険もあるばかりか、レビュワーの欠陥発見能力 の低下につながってしまう。 タスクが小さすぎて実装をするよりレビュー依頼のドキュメントを書く時間の方 がかかる様なことは本末転倒だが、数時間から長くても一日で実装が終わるくら いの大きさに設計を分割する。 変更がちょうど良いサイズになる様にするとレビューが楽になる。 実装の途中で設計に不備が見つかり変更のサイズが大きくなりそうな場合は相談 して分割できないかを検討する。
30. マスター タイトルの書式設定 コメントするけどLGTM looks good to me with commented コードレビューを楽にするプラクティス ラベルを使う際に対応が必須かどうかを一緒に書く。 [Q | 回答が必要] [MUST | 修正しないと承認できない] [IMO | 採用してもしなくても良い] [NITS | 無視して良い] [FYI | アクションは不要] IMO,NITS,FYIはコメントとしてつけるが、LGTMとしてApproveしてしまう。 採用されて複数回Approveすることもある
31. マスター タイトルの書式設定 テストコードは書く/書かせる should write test コードレビューを楽にするプラクティス 「急いでいるのでテストコードは後で書きたい」というレビュー依頼を受けるこ とがある。 しかし、結局テストコードがないことによる手戻り、バグの混入、手動での確認 に時間が取られて結果として急ぐどころか遅くなる。 テストコードを書く時間がないのではなく、単純なスキル不足なことがほとんど なので、実装にかかるコストとして受け入れた方が良い
32. マスター タイトルの書式設定 本当に必要なことだけ only essentials コードレビューを楽にするプラクティス レビューをするコードの変更が多くなればなるほど、レビュワーの負担は認知負 荷は増え、リードタイムが伸びてしまいます。 ついでにここも直したいという気持ちが湧くこともあるかもしれませんが、別の ブランチを切って別のレビュー依頼として出し直す様にすると、お互いが幸せに なれます。
33. マスター タイトルの書式設定 デイリーレビュー daily review コードレビューを楽にするプラクティス 変更がちょうど良いサイズになる様に設計しても、日をまたいで実装に時間がか かってしまうことがある。 途中でも良いのでコードの実装状態を共有しておくと中間レビューの様な役割を 果たせるので、問題となりそうな箇所を早めに見つけることができる。 あくまでも実装の途中なので細かく見過ぎない方が良い。
34. マスター タイトルの書式設定 レビュワーはデバッガーではない reviewer, not debugger コードレビューを楽にするプラクティス コードレビューはコードの品質を上げて欠陥を減らす側面があることは間違いな いが、バグを見つけることが主目的ではない。 要求どおりの価値を提供できていて他の価値を損なわないことが満たされていれ ば、軽微なバグは後で直しても良い
35. マスター タイトルの書式設定 レビュイーを尊重する respect reviwee コードレビューを楽にするプラクティス コードレビューはチェンジセットを拒否することが可能だが、それはコードレ ビューの関係性においてレビュワーがレビュイーよりも偉いことを意味しない。 （たとえ職位が上だとしても） 要求を満たすために複数の実装の選択肢が存在していても、ガイドラインから外 れていないのであれば、レビュワーの嗜好よりもレビュイーの選択を尊重する。
36. マスター タイトルの書式設定 コードレビューの類型 • グリーンフィールドレビュー • 完全に新しいコードのレビュー • 新しいコードまたはプロジェクトは、コードレビューとは別に相当な 量の設計レビューを経ることが求められる。コードレビューは、過去 に既に行われた設計上の決定について討議するための時間ではない • 挙動の変更、改善、最適化 • バグ修正とロールバック • リファクタリングと大規模変更 • 機械の生成した変更であってもレビューを要する Googleのソフトウェアエンジニアリング (p.213-)
37. マスター タイトルの書式設定 コードレビューの類型によってレビュー形態を変える グリーンフィールドレビューの様な新しい実装のレビューは、完成をしてい なくても、例えばペアプログラミングやソフトウェアチーミング(モブプログ ラミング)の様な継続的なコードレビューを行うことで、早期の相談などがで きる。 一方で対面でのレビューはレビュイーに対する忖度や場のフォースが働くた め、レビュワーの欠陥発見能力が低くなる。 バグを発見するためには、自分のペースでコードを眺める時間を作るべき。
38. マスター タイトルの書式設定 今後コードレビューで試してみたいこと
39. マスター タイトルの書式設定 LLMによるコードレビューの補佐 今後コードレビューで試してみたいこと LibSassをDartSassに変更する実装をした際に出力結果をLLMに比較してもらったと ころ、目でdiffを追うだけではわからなかった事も見つけてくれた。 変更があったドメインロジックだけを抜き出すなど、使い道はいろいろありそう なので、LLM利用を自動でかけることでコードレビューが楽にならないか考えて みたい。
40. マスター タイトルの書式設定 コードレビューのKPIにこだわる 今後コードレビューで試してみたいこと 今回の登壇をするにあたって、リードタイムの計測などを試みてみたが上手くと れなかったので、メトリクスを計測していく仕組みを作りたい。 コードレビューのリードタイムはまだまだよくなりそうな肌感もあるので、改善 もあわせて行なっていきたい
41. マスター タイトルの書式設定 一番大事なのはビジネス価値を産み出すこと すばやくリリースをして フィードバックサイクルを機能させること ボトルネックになりがちな非同期コードレビュー
42. マスター タイトルの書式設定 コミュニケーション シンプリシティ フィードバック 勇気 リスペクト エクストリームプログラミングの5つの価値
43. マスター タイトルの書式設定 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/