弁護士ドットコム株式会社 Creators’ blog

弁護士ドットコムがエンジニア・デザイナーのサービス開発事例やデザイン活動を発信する公式ブログです。

長くなりがちだったコードレビューを改善した話

弁護士ドットコム クラウドサイン事業本部でエンジニアをしている山田です。 主にフロントエンドを担当しています。

普段の業務でフロントエンド開発のコードレビューをすることが多く、今回は長い時間がかかりがちだったコードレビューを以下の施策で改善した話をします。

  • タスクへの認識合わせを拡充
  • タスクを小さく分割
  • 類似するタスクのレビュー内容は共有
  • 必要に応じて同期的にレビュー

達成されないスプリントゴール

クラウドサインでは複数の開発チームがあり、私の所属するチームではスクラムを採用しています。 スクラムでは各スプリントごとに「〇〇画面に××コンポーネントを組み込む」「〇〇画面で書類送信者が△△できるようになる」のようなゴールを設定し、チケットの優先順位や進捗の目安にしています。

ところが設定されたスプリントのゴールを達成できないことが多々あり、課題を感じていました。

スプリントゴールが達成できない原因

スプリントゴールが達成できない要因はいろいろ考えられますが、 その1つとして、コードレビューに時間がかかっているという実感がありました。 例えば1日〜2日で終わりそうな内容でも3日〜4日、ときにそれ以上かかっているような状態です。

タスクの担当者がコードレビューを依頼した後、レビューが行われ、タスク担当者がレビューに対応し、最終的にレビュアーが LGTM を出すまでの期間が長いということです。

コードレビューが長引くことで、スプリント内でタスクが完了できず、翌スプリントに持ち越してしまいます。

コードレビューが長くなる要因

ではなぜコードレビューが長くなってしまうのでしょうか。 コードレビューは、レビュアーがレビューを行う期間と、タスク担当者がレビューでの指摘に対応する期間とに分けて考えることができます。

レビュアーのレビュー期間が長い

  • レビュアーがレビュー以外のタスクで忙しく、レビューを行うまでに時間がかかる
  • レビュー依頼が多かったり特定のレビュアー依頼が集中しているためレビューを行うまでに時間がかかる
  • レビュー内容の難易度が高い、変更量が多いなどでレビュー自体の時間がかかる

レビュアーが忙しい問題は効率的にタスクをこなしたり、可能なものは他の人に依頼・移譲するなどで改善できそうです。

レビューの数が多い場合はレビュー可能なひとを増やす取組を行ったり、特定のレビュアーに集中している場合は他のレビュアーに依頼を回すことなどで解決できそうです。

レビュー内容の難易度が高いのは、1つのタスクで考慮すべき点が多かったり、そもそもレビュアーとしてどういうコードであるべきかが分かりづらい内容になっている場合などでしょうか。 コードの変更量が多いと忙しいレビュアーの場合レビューするまでの時間もかかりそうです。

タスク担当による対応期間が長い

  • レビューによる指摘事項・修正箇所が多い、修正範囲が大きい
  • レビューによる指摘事項がうまく伝わらず、手戻りが発生する

修正箇所が多かったり、修正範囲が大きいのは以下の理由が考えられます。

  • 実装方針が合っていない
    • 組織の開発方針やコーディングルールに沿っていない・慣れていない
      • リンターやフォーマッターの導入で自動的に解決できる方法もありそう
    • 変更すべきファイルやコンポーネントが方針と合っていないなど
  • 言語理解や知識の問題
    • 考慮が抜けていたり、バグになりそうな箇所があったり、もっと簡潔なコードで書くことができるなどの指摘
  • コードの変更量が多い
    • レビュー対象となるコードが多いため、指摘も相対的に多くなる

指摘事項がうまく伝わらず手戻りが発生する問題の原因は以下の理由が考えられます。

  • テキストでの伝達のためか指摘事項の細かいところまで伝わりづらい
  • 開発に参加して間もないなどでレビュアーの意図とタスク担当者の受け取り方や対応がズレてしまう
    • 指摘と併せてより具体的な対応まで伝えることで解決できそう

開発チームには、エンジニアとしての歴の長い方・浅い方、組織に参加されて長いエンジニア・間もないエンジニアなど、キャリアや状況のさまざまな方がいらっしゃいます。 どのような状況の方でも開発方針に沿いつつ迷いなくタスクを進められるようにできたら指摘事項を少なく・手戻りも小さくなるのではと思いました。

また「コードの変更量が多い」というのはレビュアー側の時間が掛かっている要因にもなっています。

対応策

これら要因に対して、以下の対応策を講じました。

  • 次スプリントのタスクについての認識合わせの時間を設ける
  • そこから見えてきたタスクの曖昧だった部分を解消するためにタスクをなるべく小さく分割
  • 類似する複数のタスクはレビュー内容を共有
  • 補足が必要な場合など必要に応じてオンラインミーティングなどで同期的にレビューする

タスクについての認識合わせの時間を設ける

まずタスクについての認識合わせの時間を設けるですが、 スクラムではすでに次スプリントのタスク整理をする Backlog Refinement を毎週行っておりタスク内容の精緻化、認識合わせもその時間で行っていました。

ですが組織に参加して間もないエンジニアとしては具体的にどのディレクトリのどのファイルをどう実装したらよいか、までの具体的な理解までその限られたその時間だけでは至らないことがありました。 タスクを着手してから自身でそれらを自分で探して作業し、コードレビューを出すことになり、作業したファイルや実装が組織の方針やレビュアーとの間でずれがありました。 その結果指摘が多くなったり、手戻りが大きくなったりすることがありました。

自分はフロントエンドのレビューを担当していたので、チーム全体での Backlog Refinement とは別に、同じくフロントエンドエンジニアと次スプリントに予定されているフロントエンドタスクをより具体的に認識合わせを行う時間を毎週設けました。

この時間のなかで具体的には次スプリントのタスクの主に以下にことついて認識合わせを行いました。

  • 作業するべきディレクトリ・ファイル
  • 該当する設計
  • 参考になる既存実装
  • タスク同士の依存関係(ブロッカーなど)
  • そのほかタスクを進めるにあたっての疑問点

またその内容をタスクの内容やコメントとして残したりしていきました。

このおかけで作業すべきファイルや実装方針に迷いやずれが少なくなり、コードレビュー時の指摘が少なく・手戻りも小さくなっていきました。

またこういった時間を設けていく中であらためてタスクの内容をより具体的にして聞くことが大事だなと感じました。 とくにタスクを進める人の目線で見直していくと、もともとのタスク内容の曖昧さや足りていない作業、ひとつのタスクでやることの多さを感じるようになりました。 タスク内容が曖昧なため作業を進めていく中で迷う部分が出てきますし、結果手戻りが大きくなってしまう原因になっていました。

タスクをなるべく小さくする

そこでもう 1 つの対応策タスクをなるべく小さくすることを実践していきました。

例えばもともと以下の2つのタスクチケットがあったとします。

  1. オプション申し込みコンポーネントを作成する
    • やること : ボタンコンポーネントを作成し、ボタンを配置したコンポーネントを作成する
  2. 上記で作成したコンポーネントを画面に組み込む
    • やること : コンポーネントを画面に配置し、申込リクエスト API モジュールを作成する、ボタンがクリックされたときに API リクエストをする、リクエストエラー時にアラートを表示させる

これを以下のようにより具体的な作業レベルにひとつのタスクの単位を小さくして進めるようにしました。

  1. オプション申し込みボタンコンポーネントを作成する
  2. オプション申し込みコンポーネントを作成する
  3. オプション申し込みコンポーネントを画面に組み込む(配置するのみ)
  4. オプション申込リクエスト API モジュールの作成
  5. オプション申し込みボタンがクリックされたときにリクエスト API を呼ぶ
  6. リクエストがエラーだった場合にアラートを出す

タスクを分割することにより前述の認識合わせの時間をやっていく中で見えてきたタスクの不明点や曖昧な部分を解消するようにしました。

その結果、タスクの担当者としては作業内容がより明確になり作業着手の心理的なハードルが下がったり、実際の作業も迷いなく進めることができるようになりました。このおかげもありレビュー段階で実装方針が変わり大きく手戻りしてしまうことが減りました。 これは特に組織に参加されて間もないメンバーには効果があるように思いました。

レビュアーとしてはレビュー対象のコード量が少なくなったことで業務の細かく空いた時間でこまめに見ることができ、早くレスポンスを返すことができるようになりました。 レビュー内容の難易度の面でも変更範囲が小さくなることで考慮すべき点やレビュー観点が明確になりレビュアーの負担も軽減されました。

タスクの担当者、レビュアー双方への効果からコードレビューにかかるリードタイムを短くできました

さらにチケットを分割することによって複数担当者が並行で作業に着手できる部分もあり、効率的にスプリントを進められることができました

またスクラムのバックログを見て何が終わって何が終わっていないか、スプリントの進捗がより細かく分かるようになった効果もありました。

タスクの担当者、レビュアー、スクラムとまさに三方よしな効果がありました。

類似する複数のタスクはレビュー内容を共有

これはタスクを小さく分割したことにも関係するのですが、少し違うが似たような実装をするタスク=類似性のあるタスクがあります。

例えば「オプション申し込みボタンコンポーネントを作成する」というタスクに対して「オプション解約ボタンコンポーネントを作成する」というタスクがあった場合。 「オプション申込リクエスト API モジュールの作成」というタスクに対して「オプション解約リクエスト API モジュールの作成」というタスクがあった場合などです。

それぞれボタンのラベルや API のエンドポイント、HTTP リクエストのメソッドが異なったりしますが作業としては類似するものです。 そういったタスクで担当者が異なる場合などはレビューの指摘内容を共有したり、複数人で同期的にレビューし、その内容を把握したうえで実装を揃えて作業してもらうことで効率的にレビューを終わらせることができました。

必要に応じてオンラインミーティングなどで画面共有し会話しながら同期的にレビューする

非同期のテキストのみでの指摘では時に意図がまさしく伝わず何度か手戻りが発生したこともありました。 場合によってはオンラインミーティングなどで画面共有し口頭で意図を補足しながらの同期的なレビューで無駄なく進めることができました。

効率とは別の側面で、テキストだけでは時に冷たい印象を受けるレビューので指摘ですが、温度感のあるコミュニケーションでのレビューはそこを和らげてくれる効果もありそうです。 組織に参加されて間もなく、まだ信頼関係が出来上がってない方とのやりとりや手戻りが大きくなるような場合では同期的にレビューするとよいことが多いかなと感じました。

スプリントゴールも達成できるように

本来スクラムイベントの中で Backlog Refinement の時間があり、タスクの内容はそこで十分に認識合わせできるのが理想的です。 ですがもともとのチケットの粒度が大きいものだと限られた時間の中では Refinement しづらい部分もありました。

タスクの粒度細かくすることで Refinement 外の時間での認識合わせもより補助的になっていきました。 コードレビューが長くなりがち、延いてはスプリントゴールが達成できない要因を徐々に解消できていった感触がありました。

スクラムチームはさまざまな背景・能力を持つメンバーで構成されています。なのでなるべく誰でもタスクが遂行できるようにチケットの粒度を細かくしたりすることで明確にすることが必要なのだと、今回の原因の深掘りと具体的な施策を通して理解できました。

まとめ

  • スクラムでの目標が達成されない原因を考える
  • 原因解決はまずできることから行う
  • コードレビューが長引いている要因を洗い出す
    • レビュアーに大きい変更量を見る時間がない
    • タスクでやることが不明瞭
  • タスクの明確化・認識合わせを、まずはコミュニケーション、そしてチケットの細分化で行った
    • チケットの細分化=タスクの明確化でレビューのレスポンス性や指摘事項の減少ができた
  • チケットの細分化により効率的なタスクの着手や進捗も見えやすくなった
  • 似た内容のタスクはレビュー内容を共有する
    • 実装を効率的に揃えることができる
  • 指摘事項をきちんと伝えるために時に同期的にレビューすることも効果的
    • 細かい意図や補足を伝えることができる
    • 関係性の構築や、テキストだけでは難しい温度感のあるコミュニケーションしやすい

以上、今回のスクラムでのコードレビューの課題を解決したお話でした。