#94 コードレビューされる技術～チームを停滞させない仕事の進め方～（2024/06/05）
こんにちは。ITベンチャーエンジニアのこへいです。
チームで仕事をしていると、仕事の進め方が下手な人が一人混じるだけでチーム全体の生産性がガクンと落ちることがあります。
エンジニアの開発チームにおいてはPull Request（PR）の書き方が下手だと、レビューのコストが大きくなり、なかなかレビューされず塩漬けになりになったり、やっとレビューされても指摘事項が多いために修正→レビューの繰り返しで、マージまでに時間がかかる上にレビュアーの時間をどんどん奪っていきます。
ということで、今回はレビューされにくいPRの特徴とどうすればレビューされやすくなり、チームの生産性向上が実現できるかについて考えます。
2024/06/13追記
コードレビューされる技術をチームに浸透させるための実践方法についてはこちらをどうぞ。
〇なぜそのPRはレビューされないのか？
レビューしたくないと思わせる「変更が巨大」なPR
変更が巨大というのは二つの意味があります。
変更ファイルが多い
変更の目的が多い
変更ファイルが30ファイルを越えるあたりで、PRを見てそっ閉じしたくなります。また、変更ファイルが少なくても複数の機能開発とリファクタが混ざったようなPRはレビューされにくいです。
何をレビューしてほしいかわからないPR
レビュアーにレビューの観点を伝えられていないPRもそっ閉じされやすいです。タイトルがチケット名だけ、変更の目的や背景、変更内容が不明なPRは、そもそも何をレビューすればいいのかがわからないため、レビュー前に情報収集が必要になり、後回しにされてしまいます。
〇レビューしやすいPRの書き方
PRを適切な粒度に分割する
PRの分割をする際に変更ファイルの数にフォーカスされがちですが、重要なのは粒度です。
1万ファイルに対する関数名の変更であればPRを分割する必要はありません。テストやビルドが通っていればレビュアーの役目は関数名が適切かどうかの判断だけになるので、レビューはとても簡単です。
では適切な粒度とは何かというと、一つのことだけに注力しているということです。
開発中にリファクタしたい部分を見つけてもぐっと我慢して、別のPRを作成しましょう。
また、PRを適切な粒度に分割するには、開発タスクの適切な分割が必要です。
例えば画面に一つの機能を追加する場合に、機能を追加するという目的は一つのように見えますが、DBへの項目追加、バックエンドへの項目・処理の追加、フロントエンドへの機能追加、外部APIやデータ出力バッチなど他の機能への項目追加などを伴う場合、一つのPRにまとめてしまうと変更ファイルも粒度も大きすぎます。
それぞれの開発スコープごとにPRを分割すべきでしょう。
つまりPRの分割のためには、開発着手の前のタスク作成時点で適切な粒度に分解しそれぞれの修正をマージしてもアプリの後方互換を保てるような進め方の設計が必要になります。
レビュー観点を明確にする
変更の目的、変更内容、変更によって期待される結果を明確にしましょう。
変更の目的がパッとわかるようなタイトルを付け、PRの概要にその変更が必要になった背景を簡潔にまとめましょう。変更の経緯などをまとめたチケットへのリンクも必要です。
〇変更の目的
背景がキチンと共有されることで、その要求を満たすのであれば機能追加でなく既存機能の活用でも十分とわかり不要な機能の追加を防げることもあります。（本来は開発着手前に気づきたいですが。。。）
またプロダクトの文脈を全員が把握していることは稀なため、レビュアーに文脈のキャッチアップを強いるとレビューが停滞します。初見の人が前提知識がなくても理解できるように書きましょう。
〇変更内容
変更内容を箇条書きでよいのでまとめましょう。
ソースを読めば変更内容はわかりますが、先に言葉で内容を理解してからコードを見る方がコードの意図に対して適切な書き方かどうかの判断がしやすいです。
また、特にレビューして欲しい点や、実装の参考にした記事へのリンクがあれば明示的に残しておくのが望ましいです。
特殊な書き方をしている場合に、レビュアーの負担を減らすことができます。
変更内容を書くのに苦労する場合はPRの粒度が大きすぎることが考えられます。その場合は面倒に感じても分割するのが望ましいです。
〇変更によって期待される結果
レビュワーがコードを把握する上で、正しい挙動はどのようなものか、を理解しておくことは大きな助けになります。
APIのリクエスト／レスポンスや、画面上の表示や振る舞いがわかるキャプチャを貼っておくことで、レビュアーは変更の理解が容易になります。
また、開発環境での再現手順やURLも記載することで、レビュアーが自分の手元で再現し動作を確認することも出来ます。
粒度が単一でも複雑な実装・振る舞い実装となる場合には、特に役立ちます。
これらの観点をPRに記載するのは大変に感じるかもしれません。
しかし、自分がレビュアーになったつもりで自分のPRをレビューしてみてください。これらの項目が必要であることが理解できると思います。
〇レビューする時間がないという課題
PRレビューが滞留するという議論の際に必ず「レビューする時間がなくてレビュー出来ません。」という意見が出てきます。
レビューのコストは高い、まずこのことを認識しないといけません。
本当に真面目なレビューを行うと仕事時間の30%はコンスタントにレビューに持っていかれます。開発メンバー10名程度のPRを全てレビューしていた時はもっと多いかもれません。
コードを書くことだけが仕事と考えてしまうと結局マージされないためアウトプットは無いことになります。
また、レビューとセットでアウトプットが作られるのに、レビューのコストを甘く見ているとプロジェクトは遅延します。遅延を防ぐためにレビューされなずにマージされるようになると、もはやレビューは機能していません。
そして、デリバリーを優先することで、プロダクトの品質が犠牲になります。
良いレビュー体制を築き上げるにはまずレビューにかかるコストを理解し、レビューを行う利点、エンジニアがレビューを行うことを奨励しているということを積極的に周知する必要があります。
ということで、開発組織の生産性を向上するためのレビューされやすいPRを作る技術、そしてレビューを機能するための体制作りについての話でした。
今日も最後までお読みいただきありがとうございます。
参考記事：
コードレビューについて
レビューしてもらいやすいPRの書き方
Findyの爆速開発を支えるテクニック
レビュワーを"憑依"させて Pull Request をセルフレビューする