コードレビューをラウンドロビン方式に
こんにちは！AndroidエンジニアのYukiです。
プログリットでは、事業の拡大に伴い、モバイルエンジニアの人数も着実に増えてきています。
約2年前には、各OSに2名ずつ配置され、合計4名でモバイル開発を進めていた時期がありました。
しかし、現在ではその人数が10名に増え、倍以上の規模となりました！
これは非常に喜ばしいことですが、人数の増加に伴い、これまでの開発プロセスを見直す必要性が生じてきました。
その一環として、モバイルチーム内でのコードレビュー体制を改善するために、Githubの機能 を使い、ラウンドロビン方式 を採用しました。
今回は、このラウンドロビン方式の導入に至った背景、導入後の結果についてお話ししたいと思います。
今まで何が起きていたのか？
まず、プログリットでは、以下2つの観点で、コードレビューを行っておりました。
機能面
例: 仕様を満たした実装になっているか、デグレが発生しないか。
コード品質
例: アーキテクチャの違反がないか、可読性が低くないか。
プログリットでは各プロダクトに1名ずつエンジニアが配置されていますが、別プロダクトの機能面のチェックを行おうとすると、以下の問題が発生していました。
インプット不足：どのような機能が実装されるのかを把握するために、Figmaなどの資料を確認する必要があり、レビュー負荷が高い。
影響範囲の把握困難：携わったことのないプロダクトでは影響範囲を適切にチェックすることが難しい。
従って、上記2つのチェックができる人が限られるため、特定のメンバーにレビューが集中し、以下のような問題が発生していました。
Pull Requestの滞留時間が増加し、レビューイーの作業がブロックされる。
レビュワーはコードレビューの数が増加し、自身の作業が進まなくなる。
参考までに、これまでのレビュー体制を図式化してみました。これを見ると、特定のメンバーにレビューが集中している状況が一目瞭然です。（矢印の向きに、レビュー依頼をしていることを示しています。）
どうやって解決したのか？
弊社で活躍しているMobileのテックリードであるTakahiroさんを中心に体制づくりを進めました。
コードレビュー体制の叩き台を作成した上で、業務委託の方を含む各所からフィードバックを募り、チーム全体で「コードレビュー体制」を検討しました。（社内でのやりとりの一部を公開しちゃいます！）
検討を重ねた結果、「 特定のメンバーに集中していたレビューを他メンバーに分散させたい」という目的を達するために、「職能ごとでのラウンドロビン方式」を採用しました。
ラウンドロビン方式を採用する場合の懸念は？
別プロジェクトの背景知識を知らない中で、約5名で構成される職能チーム内で、レビューをランダムに回すのは、かなりチャレンジングな活動だと捉えています。
従って、「明日からラウンドロビン方式に変更です！」とはならず、まずは、色んな懸念を払拭していく必要がありました。
機能面のチェックはどうするの？？
今までは、ある程度、プロダクトの背景知識のあるメンバーがコードレビューを担当していたので、コードレビューの段階で、バグを潰す、ということが可能でした。
しかし、ラウンドロビン方式により、プロダクトの背景知識のないメンバーもレビューに加わり、結果、コードレビューでは機能面を担保できない、という懸念が出てきました。
議論の結果、コードレビューでは機能面のチェックは「必須」としないことにしました。（もちろん、パッときづいたり、チェックをしたいなら、それでも良い、というスタンスです。）
プログリットでは、一部のプロダクトで、スクラム開発を導入しており、スプリントレビューで機能面の担保はできます。
また、各プロダクトにQAさんを配置しており、クリティカルなバグは検知できる仕組みが備わっています。
さらに、機能面のチェックをできる仕組み自体は存在しているのですが、それに加え、エンジニア内でも機能面の担保をできる仕組みづくりに取り組んでいます。
例えば、「単体テストの拡充」です。単体テストを記述する文化がなかったわけではないですが、単体テストを行うかどうかは開発者に委ねられています。今は特定のメンバーのみ、単体テストを記述しているのですが、ゆくゆくは、モバイルチーム全体に浸透していくよう、Pull Request のコミュニケーションを活用しています。
他にも、「Given-When-Thenシナリオ作成」を現在進行形で取り組んでいます。詳しくは、弊社 モバイルテックリードの Takahiroさんの 記事をご覧ください。
プロダクト間で設計方針がブレないか？
今までは、特定のメンバーがレビューを担っていたが故に、設計方針が各プロダクトでばらつきにくい、というメリットがありました。
しかし、ラウンドロビン方式により、異なる価値観を持っているメンバー同志によって、あちこちでコードレビューが繰り広げられるわけです。
したがって、設計方針やコーディングスタイルが統一されず、コードの一貫性が低下する危険性もあります。
こちらに関しては、「最初から完璧を目指すのではなく、徐々にチームで洗練させていこう」という方針のもと、現在進行形で、以下の取り組みを行っています。
ADR（Architecture Decision Record）で、チーム間で設計方針を揃えていく。
2週間に1回の「モバイル振り返り会」にて、設計に関する課題があれば、みんなで議論し、ADRに落とし込む。
ちなみにですが、以前のモバイル振り返り会にて、「ViewModel から Repository のメソッドを呼び出していいの？」って議題があがり、各々の考えを共有した上で、チームとしての方針を固めていました。
各所で意見の衝突が起きないか？
文化や背景が異なるメンバー同士が相互にコードレビューを行うので、意見の衝突が起きたり、議論がまとまらない、という事態も起こりかねません。
この点については、「コードレビューのガイドライン」を策定し、レビューの基本姿勢や判断基準をチームで共有することで対応しています。ガイドラインを通じて、建設的かつ公平な議論が行える環境を整えています。
コードレビューのガイドラインの一部を紹介します！
さらに、2週間に1回の「モバイル振り返り会」を設け、発生した課題や懸念を議題として取り上げ、チーム全体で解決策を検討する仕組みを整備しています。
結果、どうだったのか？
まず、新しいコードレビュー体制を2週間運用してみての、チーム全体の反応としては、かなりポジティブでした。笑
また、弊社では、試験的に、PRに関するメトリックスをモニタリングしており。新しいコードレビュー体制の導入前と導入2週間後での結果を見ても、ほとんどの項目において、良い数値が出ています。
元々の課題であった「PRの滞留時間が増加し、レビューイーの作業がブロックされる」という問題は、大幅に改善されました。この改善は、特定のメンバーに集中していたレビューを他のメンバーにも分散させたことが主な要因だと考えられます。
また、各PRのコード変更量も以前より小さくなっています。これは、プロダクトの背景知識がないメンバーにもレビューを依頼する必要があるため、「PRサイズをできる限り小さくしよう」という意識が働いた結果だと考えられます。
一方で、「レビュー中に残されたコメント」の数はやや増加傾向にあります。しかし、プロダクトの背景知識が不足している中でレビューを行うため、ある程度は仕方ないと考えています。また、PRの滞留時間が短縮されていることから、この点は大きな問題ではないと見ています。
機能面の品質が十分に担保されているかについては、現在のところメトリクスで測定できていません。しかし、コードレビュー体制を変更したことによる致命的なバグは今のところ発生しておらず、一定の信頼性は保たれていると判断しています。
以上のことから、ラウンドロビン方式を採用したことは現時点では成功と評価できます。ただし、これで終わりではなく、2週間に1回実施しているモバイルチームの振り返り会を活用しながら、現状を見直し、コードレビュー体制をさらに洗練させていきたいと考えています。
最後に
ここまで読んで頂いてありがとうございました！
プログリットでは、絶賛モバイルエンジニア採用中です！
カジュアル面談も受け付けておりますので、少しでも弊社にご興味を持たれましたら、お気軽にお申し込み頂ければと思います。
