「とりあえずやってみよう」で育てる。とある開発グループのレビューフロー改善記

多様な開発組織のコード／PRレビューを“図鑑”のように紹介する連載「コードレビュー図鑑」。今回は、株式会社Works Human Intelligenceの丸山さんに、最適なレビューフローを探り続けた約2年の試行錯誤を伺いました。実践してきた4種類のレビューフローを、メリット・デメリットとともに振り返ります。

はじめに

はじめまして。株式会社Works Human Intelligenceで開発マネージャーをしている丸山（@moromi25）です。
育児時短勤務をしながら、所属する開発グループのグループ運営、開発メンバーの育成、担当機能群のリリーススケジュールや実際のリリース内容のレビューなどを担当しています。
いくつかのプロダクトや開発グループへの異動を重ねており、今のグループにはプレイヤーとして異動して約1年、その後マネージャーになって約2年が経過しました。

私たちの開発グループでは私がマネージャーになって以降、プルリクエスト（PR）のレビューについて、チーム状況に応じた最適なレビューフローを求めて、約2年半にわたって議論しながら改善を続けています。

本記事では、これまでにどのようなレビューフローをとってきたか、またその中でどういった点がよい／課題と感じ、さらなる改善にいたったのか、また改善をするためにマネージャーとして開発グループにどう働きかけているかをご紹介します。

PRレビューで大切にしていること

私たちのグループでは、PRレビューおよびレビューフローにおいて以下を大切にしています。

• 製品やコードの品質を担保するための場であることは大前提である
• メンバーの成長の場である
• 滞留せず、快適に開発を進められる
• "今"のレビューフローに納得感が持てる
• "今"のレビューフローに固執しすぎず、以下を加味しながら柔軟にフローをアップデートしていく
  • チームメンバーの人数やスキル
  • チームが持つ担当機能群のフェーズ

実践してきた4つのレビューフローのメリット／デメリット

現在のレビューフローに至るまで、そのときの課題やチーム状況に応じて試行錯誤を繰り返してきました。

レビューフローA：マネージャーが全チームの全PRをソロレビュー

私が前任のマネージャーから開発グループのマネージャーを引き継いだ時点でとっていたフローは、マネージャーが全チームの全PRをひたすら1人でレビューしていくやり方で、しばらくはこのやり方を踏襲していました。

メリット

開発メンバーがPRレビューに時間を取られない

PRレビューはそれなりに時間と集中力を要します。このフローであれば私以外の開発メンバーはPRレビューに時間を割く必要がないので、各々の開発業務に注力することができます。

デメリット

マネージャーがブロッカーになり、レビュー遅延が発生しやすい

唯一のレビュワーであるマネージャーがブロッカーとなり、レビュー遅延が発生しやすくなります。
マネージャーである私がPRを見る時間を取れなかったり、開発締め間際でPRの提出が集中したりすることで、容易にレビューが滞留しやすい状態が生まれました。
また、プレイヤー時代に担当していなかった機能もレビューすることになったため、私自身のドメイン知識や担当機能群のコード把握が浅いことも、レビュー遅延に拍車をかけていました。

レビューという最良の学習機会をマネージャーが独占してしまう

レビュー遅延を申し訳なく思ったり、負荷が高いと感じている一方で、私個人としてはすごく勉強になりました。PRを通して日々いろんな知識をメンバーから学べますし、レビューしやすいPRも見えてきます。
この最良の学習機会をマネージャーが独り占めしてしまっているのは、グループ全体にとって大きな損失だと強く感じていました。

レビューフローB：全PRを全チームメンバーでモブレビュー

フローAのデメリットはいずれもマネージャーが1人でPRレビューを進めていることが要因です。
この状況を変えるべく、すべてのPRをチームメンバー全員で集まってレビュー、つまりモブレビューに切り替えることにしました。

モブレビューの進め方

• 朝会の最後のアジェンダとしてモブレビューの時間を設ける
• 朝会までに上がったPRの担当チームが残り、マネージャー＋チームメンバー全員でモブレビュー
• レビュイーが簡単にPRの内容を説明した後、その場でレビュアーがコメント／マージ

メリット

1営業日以上のレビュー滞留がほぼなくなった

これが何より大きかったです。朝会までに上がったPRを朝会の中で見るため、すぐにレビューされて開発サイクルを前に進めることができます。
開発締めが近くなると、朝会ではレビューしきれない数のPRが上がってきたり、両チーム一斉にレビュー依頼が舞い込んだりするので、そのときは夕会で延長戦をしたり、別途追加のレビュー時間を押さえてとにかくみんなでレビューをしました。

メンバーがレビュー経験を積めたうえに、レビュイースキルも上がった

レビューする立場になると、以下のようなことが見えるようになってくるため、メンバー全員がレビュアーとしての視座を得るだけでなく、レビュイーとしてのPR作成の質も底上げされました。

• あらかじめPRの説明文やコード上の補足として書かれているとレビューしやすい内容
• レビューしやすいコミットの分け方／PRの大きさ／PRの粒度

チーム全体で、担当領域への理解が深化した

レビュー対象のPRの内容はもちろん、それまで見えづらかったメンバー同士の担当案件の内容や、メンバーが持っている機能仕様や業務ドメインの知識も共有されて、チーム全体の担当機能への理解度が底上げされました。
自力で調べようとしたら途方もない時間や労力が必要な内容も、ベテランメンバーたちから若手メンバーたちに伝播されていくので、レビューの速度も上がりました。

コーディングルールの確認・醸成

コーディングルールをその場で確認し合ったり、議論が巻き起こるようなものはその場でルールを作ったりできました。

レビュアーとして「PRを見る時間を作る」というタスクが減った

このやり方はマネージャー自身がPRレビューをすべてやるという点で、負荷は変わらないように見えます。しかし実際は、毎日PRを見る時間が強制的に朝会に組み込まれたことで、PR見る場を作るというタスクが減ったのですごく楽になりました。
マネージャーはミーティングが多くなりがちなので、その合間を縫ってレビュー時間を確保するのはなかなか大変です。それが勝手に組み込まれて、舞い込んでくれるのは想像以上に負荷が軽くなります。

レビュー時の観点漏れを防ぎやすくなった

レビューでOKを出したものは、世に出ていきます。以前はどこでOKを出すかはすべて自分にかかっており、かなりのプレッシャーでした。責任ある立場として、あそこも確認しなきゃ、あっちは大丈夫かな、と心配が尽きず、結果的にレビュー完了が遅れる要因の1つにもなっていました。

もちろん、モブレビューを始めたからといってPRとして上がってきたものをOKする責任が変わるわけではありません。しかし、メンバーと協力しながらレビューを進めていけるようになり、観点漏れを防ぎやすくなりました。

デメリット

表面的なフィードバックに終始しやすくなった

事前にPRのレビュー依頼がSlackで上がってきていても「モブレビューの時間に見ればいいか」となり、モブレビューで初めてPRの中身を見ることが増えました。
その結果、表面的なコーディングの指摘に終始しやすくなりました。

PRの中にはソースの前後関係や機能全体の中での役割を汲んだうえでレビューが必要な内容もありました。そういったPRはレビュー前に内容にざっと目を通し、必要に応じて手元でコードを見るなどしてからモブレビューに臨むようにしないと、レビューとしてあまり意味をなさなくなってしまいます。
しかし事前に声掛けなどをすることもなく、一律でモブレビューとしてしまっていたため、上記のような下準備をするかどうかはレビュワー任せでした。
そのため、PR上の修正内容のみに指摘がとどまりがちになりました。

レビューフローC：全PRを全チームメンバーで非同期レビュー

フローBを経たことでレビューの民主化が進み、メンバーのレベルの底上げも一定実現できました。しかしデメリットに挙げたとおり、各レビューが浅くなりやすい状況になりました。これでは本末転倒です。
次第にメンバーからも「PRをもっとちゃんと見たいです」「レビューのやり方変えませんか？」という声があがってきました。

そこで、すべてのPRを全チームメンバーで見ることは変えないまま、モブではなく各々のタイミングでレビューを実施する非同期レビューに変更しました。

非同期レビューの進め方

• レビュアーが各々のタイミングでレビュー
• 全レビュアーのApproveをもってマージ
• レビュイーに解説してほしいPR、他のレビュアーと議論したうえでApprove／コメントしたいPRは朝会でモブレビュー開催

メリット

じっくりとレビューできるようになった

PRそのものをじっくり見られるようになりました。また、必要に応じてそのソースコードのブランチをpullして前後関係をIDE上で確認したうえでApprove／コメントすることができるようになりました。

チーム内のレビュー観点の醸成やノウハウ共有もPRコメントを通して継続できた

PRコメント上でメンバー同士で議論ができていました。
また、メンバーのレビューコメントを他メンバーが見に行く、ということも意識されており、レビュー観点を作ったりノウハウを共有したりもPR上で継続できていました。

デメリット

再びレビュー遅延が発生するようになった

じっくりPRを見られるように、を重視した結果、再びレビュー遅延が発生するようになりました。

レビュイーによる口頭説明もなくなって、読み解き難易度があがってしまったこともあり、じっくり見るという名目でついついレビューが後回しになるようになりました。

全メンバーによるApproveを待つという点も特に緩めていなかったため、遅延に拍車をかけていました。
長いものだと2週間近くマージされないPRなども出てきてしまいました。

レビュー滞留によって、レビュイーはなかなか開発サイクルを次に進められず、マルチタスク状態やタスク共倒れのリスクが発生し、サイクル終盤での負荷が上がっていました。

現在実践しているレビューフロー：チームごとに最適化（モブレビュー回帰／条件付き非同期レビュー）

これまでの変遷とメンバーとの会話を経て現在実践しているのは、2つあるチームで統一のレビューフローを取るのではなく、各チームに最も合うようなフローを採用するというやり方です。

チーム1：モブレビュー回帰

チーム1は全チームメンバーで全PRをモブレビューする方法に戻しました。
チーム1が扱うドメイン領域が他ドメインとも密接に関わり、仕様も複雑です。1サイクルあたりで作成されるPR数が少ないため、非同期でレビューするよりも、メンバー同士が認識合わせをしながらレビューするほうが効率的かつ効果的だと判断しました。

チーム2：条件付き非同期レビュー

チーム2はフローCで実践していた非同期レビューを踏襲しつつ、いくつかの制約を加えました。

• 期限の制約
  • 1営業日以内の非同期レビュー^[4]（これまでは制約なし）
  • それ以降の滞留／必要に応じてモブレビュー開催
• マージまでの制約（緩和）
  • マネージャー＋メンバーの過半数のApproveでマージ（これまではメンバー全員のApproveが必要）
  • 新規配属メンバーは他メンバーと同様にレビュアーになってもらうが、約6ヶ月は過半数の対象に含まない（そのメンバーの配属までのバックグラウンドによって適宜調整）

メリット

レビュー遅延が発生しなくなった

どちらのチームも、1営業日以上のレビュー遅延は基本的に発生しなくなりました。

遠慮なくモブレビューを依頼するメンバーが増えた

チーム2のメンバーは、非同期とモブの使い分けが以前よりも柔軟にできるようになりました。
期限の制約が設けられたこともあり、メンバーが無理せずモブレビューを依頼することが増え、ちょっと読み解きが難しいようなPRも早くレビューが進むようになりました。

デメリット

“PRレビューあふれ”を起こすタイミングがある

特に開発締め前、かつ機能強化の案件や難易度の高めの不具合がいくつか重なる開発サイクルの場合、局所的にレビューが回らないシーンが出てきます。
その際は、今使えるレビュー時間、PRの大きさ、緊急度や重要度などを加味して優先順位をつけ、朝のレビュー時間で足りなければスポットでレビューの時間を入れて...と、順番にレビューをしています。そうしてなんとかレビューを回すと1日が終わる...という日も。

メンバーが成長し開発期間内に実装できる量が増えてきたことによる嬉しい悲鳴ではありますが、スポット的とはいえ“PRレビューあふれ”を起こすと、優先順位を下げさせてもらったPRはレビュー遅延が発生してしまい、開発サイクルが進まなくなってしまいます。
レビュアーであるメンバーも自分自身の開発タスクがあるので、そこから自身のタスクを進めるのはかなり体力気力が必要です。

この状況はフローBの際も起こっており、対策を考える必要があります。
そしてこの状況は、レビューフローの改善だけでは解決しないとも思っています。
自分たちが扱えるレビューの数や大きさを把握して、そこに収まる案件規模での見積もりができる体制や、そもそもPRレビューに上がる前の実装段階からお互いの開発内容を把握する体制の見直しも必要です。
具体的にどう改善していくか／しばらく様子を見るかは、引き続きメンバーと相談しながら決めていく予定です。

レビューフローを頻繁に変えることに、メンバーからの異論はないのか？

ご紹介してきたとおり、この2年間半ほどで3回にわたってフロー変更をしてきました。結論から言うと、フローを変えること自体にはほぼ異論は出ません^[5]。
これはメンバーが全員「とりあえずやってみましょう！」という姿勢でいてくれることが大きいです。
同時に私自身も、メンバーを置いてけぼりにしないフロー変更を目指しています。

メンバーからの違和感をすくいあげる

レビューフローを変えましょう！と最終的に声をかけることはマネージャーである私が多いものの、その前にメンバーからの違和感のすくいあげをしっかりするように心がけています。

違和感をすくいあげる流れは大きく2つあります。

• 自分発信：私（マネージャー）がPRレビューを実施している中で違和感を感じる→その違和感を各メンバーとの1on1などで雑談してみる
• メンバー発信：メンバーが1on1で「最近のレビューちょっとやりづらくて～」という声を上げてくれる→他メンバーにもヒアリングしてみる

いずれの場合も、最初から全員で一気に話すのではなく、メンバーと個別に会話をしながら改善に向けて動くとうまくいくと感じています。

自分発信の場合は、違和感や改善案を個別に話してすりあわせる

自分発信の場合、今自分が抱えている違和感や課題感に他のメンバーが共感してくれるとは限りません。

たとえばフローAからフローBに変えるときは、私がレビューのやり方に限界を感じ、私発信でレビューフローの改善に着手し、レビュー手順やたたき台はほぼ1人で考えました。
その後、各メンバーとの1on1で、フローAに対する課題感やフローBのモブレビューへの切り替えやその実施イメージを個別に話しました。

話したときのメンバーのレビューフローに対する課題感はそれぞれでした。
すでにソロレビューによって開発サイクルが滞ってしまうことへの課題感を同様に感じていたり、レビューを通してメンバー間で技術や業務知識を伝播することに意欲的なメンバーもかなりいました。
それ以外のメンバーも、疑問点を解消したうえで変更に賛同してくれました。

グループ全体の話し合いの場を作るほうが効率はいいですが、1人1人の発言ハードルが上がってしまいます。結果的にフロー改善はできたかもしれませんが、モヤモヤを残したままになってしまったかもしれません。個別に相談をすることで内容に対する率直な質問や意見を聞けました。
また同じ課題感があるとわかれば、地続きでフローの改善案を話せ、すばやく変更につなげることができると感じました。^[6]

このときの経緯や細かな実践内容はこちらの記事にまとめています。
開発レビューをモブでやるようにしてみたら、 - Qiita

メンバー発信の場合は、一緒に作戦会議しつつ他メンバーへのヒアリングを並行する

フローBからフローC、フローCから現在のフローに改善した際は、私が違和感を感じたのとほぼ同時期にメンバーから私にレビューフローへの違和感を伝えてくれました。

メンバー発信の場合、メンバーが感じている違和感を私自身の言葉で言語化し、認識を合わせたうえで次に進むとよいと感じています。違和感のずれも防げますし、方向性が揃った行動意志のある人間が2人以上いると、それだけで推進力が上がるためです。

主動や役割分担などの意向も確認します。
声を上げてくれたメンバーが主動したい場合はもちろんおまかせしますが、もともとグループ運営に責務があり、週次でメンバーとの1on1が一巡する私のほうが圧倒的に早くヒアリングなど進められます。そのときのメンバーのタスク状況なども加味して動き方を決めます。

ここから先の進め方は自分発信の場合とほぼ共通ですが、発信メンバーがいることで「2人目に踊る人」としてフォローしてもらえるので進めやすくてありがたいです。

迷いを与えないレビュー手順を作成する

改善案を作る際には、必ずレビュー手順を併せて考えます。
このレビュー手順には、レビューの開始から終了までのステップと、状況別の対処法をすべて記載します。
実際の手順はこちら。GitHubで管理をしています。

改善後の具体的なレビューフローのイメージが具体的に思い浮かぶと、実際の変更に踏み切りやすくなります。
また「とにかく新しく決めた手順の通りにやればOK」な状態を作っておくことで、変更後のレビューそのものに集中できます。変更後の手順に迷うポイントがあると、レビューもしつつフローも決めつつでマルチタスク化してしまい、途端に負荷が上がり、最悪改善の実感を得られずにもとの状態に戻してしまうこともありえるからです。

最低1サイクルは改善後のレビューフローを続け、同時にレビュー手順に微調整を加え続ける

どんなに具体的で完璧だと思えるレビュー手順を作ったつもりでも、実際にフローを回していくとやりづらい部分や考慮不足が見つかります。
レビュー中に「こっちの方がやりやすいですね」となったらレビューフローを調整し、併せてレビュー手順を書き換えます。

常に退路を作っておく

フローを変えるときは「まず1サイクル回してみてイマイチだったら戻そう」と必ず伝えています。元に戻せるということがわかっていれば、安心してお試し改善ができると思っているためです。

メンバーによるアップデートを尊重する

レビュー手順を作成して型は決めるものの、慣れてくるとその型で取り回しづらい場面も出てきます。
現在のフローで進める中でも、たとえば以下のようなパターンがありました。

• すべてモブレビュー開催が前提だが、軽微な修正でPRの概要に記載した内容でレビューが完結する場合は非同期レビューで済ませる。この「軽微な修正」はチームメンバー内で共通認識ができればOK
• 1営業日以内のレビューが前提だが、より高サイクルで回したいため最初からモブレビューを依頼する

レビューフローを踏襲することは手段であり目的ではありません。
メンバーが開発サイクルを前に進めることができ、製品やコードの品質が担保できるのであれば既存のフローにこだわりすぎる必要はないので、基本的にはメンバーのやりやすい方法への改善を尊重しています。

おわりに

これまで実践してきたレビューフローはいずれも珍しい内容ではありませんし、その結果起こったメリット・デメリットも振り返れば当たり前な内容も多いです。また、私たちのグループには合わなかったフローが、他の開発チームではフィットすることもあると思います。
私は、それを自分たち自身で実践し、結果を体感することに意味があると思っています。

至高のレビューフローは存在しないので、チームがPRレビューで大切にしていることは大前提に置きつつ、そのときどきのチーム状況に応じた最適なレビューフローを、これからもメンバーと一緒に模索し続けたいと思います。

※本記事は以前、「ぼくらのPRレビューフロー改善奮闘記 ～ 品質も成長も快適さも兼ね備えたフローを求めて」というタイトルで執筆した記事を加筆・修正したものです。

一緒に日本のバックオフィスを支える製品をつくりませんか？

「COMPANY」は国内大手法人^[1]の3社に1社^[2]にご利用いただき、その人事業務領域を支え続けています。
そんな「COMPANY」の開発現場では、技術選定、サービス・機能のデザイン、設計から開発、評価、リリースまで一貫して担当することができます。また前述してきたとおり、開発グループ内はもちろんのこと、グループや部署の垣根を超えた風通しの良さも当社開発組織の自慢です。職位や職能に縛られず横断的に製品開発や課題の解決に取り組むことができます。
私たちと一緒に日本のバックオフィスを支える製品をつくってくださる方、またその製品づくりを支える開発組織を一緒に育ててくださる方、お待ちしております！
ぜひ下記リンクよりご連絡ください。カジュアル面談も随時受け付けております！