なぜ開発基盤グループではPR作成者がマージを行うことにしているのか

最近、作ったプラモデルをついに秋葉原に展示していただきました、デジタルトランスフォーメーション事業本部共通開発部開発基盤グループのリーダーの西田和史です。

秋葉原のラジオ会館8Fボークスさんに12月まで飾ってあるので、近くに用がある方は良ければ見てください

プルリクエストのマージをする人の話

今日のモダンな開発はほぼプルリクエストまたはマージリクエストを使って行われていると思います。このプルリクエスト、あなたのチームでは誰がマージをすることになっているでしょうか？

よく聞くのが、チームリーダーやチームの中でそのプロダクトに最も詳しい人、そして大体の場合レビュアーの人物がマージするパターンです。それに対して、僕の所属する開発基盤グループでは僕が加入した4年前から一貫してプルリクエストの作成者がマージするようにしています。

この2つで何が違うか。僕は、その違いはマージされたコードの最終責任は誰かという点だと思っています。コードをマージするということはそのコードが本番環境にデプロイされても問題ないこと、および、そのリリースタイミングが今(次)で問題ないと承認することにほかなりません。言い換えると、そのコードについて本番環境での稼働に責任を持つ、ということになります。

マージする人 = チームリーダー(レビュアー)

マージする人がチームリーダーの場合、コードについて責任を持つのはチームリーダーです。これは一見良さそうに見えて、いくつかの欠点を持っています。

第一に、プルリクエスト作成者の成長を阻害します。コードの品質について最終的に責任を持つのがリーダーになるため、どうしても品質の低いプルリクエストが生まれがちです。正確に言えば、もっと良い品質のプルリクエストを作れるにも関わらず、コード品質の最終責任がリーダーであるために細部を突き詰めず、とりあえずレビューに出す、といったことがどうしても発生してしまいます。

第二はチームがスケールしないことです。第一の欠点から、リーダーには高強度のレビューが継続的に求められます。そのことよりメンバーを増やしてもリーダーがボトルネックとなり生産性がスケールしません。また、リーダーも時間の使い方がレビューに偏り能力開発やチームの技術的トップラインを引き上げるアクションができなくなります。そして、大体の場合はだんだんリーダーのレビューが形骸化します。構造に無理があるので必然です。

第三に、リーダーのスキルの上限がチームの能力の上限になることです。最終的にコードについて責任を持つのがチームリーダーですので、本人がわからないコードは基本的にマージできません。第二の問題からリーダーのスキルが成長せず逆にメンバーが成長して追い越してしまった場合、リーダー以外のメンバーはわかるがリーダーがわからないのであえて愚直な書き方をせざるを得ない状況が発生します。

マージする人 = プルリクエスト作成者

マージする人がプルリクエストの作成者の場合、コードについて責任を持つのはプルリクエストの作成者です。このときのメリットは、レビュアーがマージする場合の逆です。

自分の納得するコードがそのまま本番環境へ反映されるため、能力が結果に結びつきやすくプルリクエスト作成者にとって成長に対するインセンティブが高くなります。

次に、オーナーシップがプルリクエスト作成者にあるため、チームリーダーに責任が集中しなくなり、チームがスケールしやすくなります。

最後に、チームリーダーがわからないコードでも本番環境へデプロイすることが出来ます。もちろん、チーム全体の合意を取ったり、理解を共有するためにレクチャーなどはするべきですが、チームリーダーが分かる範囲でないとマージできない、といった制約はなくなり、例えばチームの過半数の人間が理解してApproveできるならマージする、ようにできます。

マージする人 = プルリクエスト作成者とするために必要な施策

ただし、プルリクエスト作成者がマージを行う場合はチームリーダーがマージを行う場合と比べていくつかの問題があります。

例えば、チーム内のコード品質の問題です。チームリーダーがコードの責任を持っている場合、チームリーダーの中で統一されたコードの品質基準があればプロダクト内のコードの品質は一定に保てます。ただし、マージする人 = プルリクエストの作成者の場合、コードの品質を担保するのはプルリクエストの作成者です。なので、チーム内全員で合意できているコードの品質基準が必要になります。チームが小規模であれば暗黙的な合意で十分かもしれませんが、一定の規模より大きい場合はLinter, コードフォーマッタ、自動スキャナなどが必須となるでしょう。

また、プルリクエストの作成者がレビューを依頼するときにレビューで見てほしいポイントを明示したり、逆にレビュアーがプルリクエストの作成者へこの部分は見ていない、レビューできなかったです、というところを明示することも重要です。チームリーダーが全コードの品質を担保し責任を保つ場合、全コードをリーダーが見ればいいですが、そうではない場合、どこが誰のレビューをOKしていてどこがだれも見ていないかを間違いなく認識する必要があります。これに気をつけないと、結局XXさんのレビューを受けたから、という責任感の放棄が発生します。ときに誰のレビューを受けていないコードをマージするためにも¹、どこを誰がレビューして、approveしたかを認知し合うのは重要です。

という感じで、開発基盤グループ内で行っているマージとレビューの考え方について紹介しました。

注意点として、実際のところこれがどの程度再現性があるのかは僕自身もわかっていないことが挙げられます。

開発基盤グループで扱っているkubernetesマニフェストやargo cd, terraformなどの道具が比較的習得しやすく、また少人数で多数のシステムを扱っているために各々が自分の担当するシステムや事業部にオーナーシップを保つ必要がある。そういった開発基盤グループの性質がこのやり方とマッチしているという点はあるかもしれません。

ただ、それを除いたとしても、僕はこの各々の責任感や能力に期待して大きな権限を持ってもらうこのやり方が好きです。また、そういったやり方はメンバーや1年目の新卒にも信頼して大きな権限を渡し、失敗を許容しつつそれを成長の糧として積極的に活用する、僕たちのデジタルトランスフォーメーション事業本部やSpeeeのあり方とマッチしています。

いつものやつ

Speeeでは一緒にサービス開発を推進してくれる仲間を大募集しています！
こちらのFormよりカジュアル面談も気軽にお申し込みいただけます！

Speeeでは様々なポジションで募集中なので「どんなポジションがあるの？」と気になってくれてた方は、こちらチェックしてみてください！もちろんオープンポジション的に上記に限らず積極採用中です！！！