そのPull Request 最後まで責任をもって作成してますか?
はじめに
この記事は エンジニア組織の開発生産性・開発者体験向上の取り組みをシェアしよう! by Findy Advent Calendar 2023 シリーズ2 の6日目の記事となります。
株式会社DELTAでエンジニアをしている長田(おさだ)です。
みなさん、ソースコード管理サービスは何を使っていますか? 私が所属している開発組織ではGitHubを使用しており、今回はPull Request(以下、プルリク)の「責任ある運用」について書きたいと思います。
端的にまとめると 「プルリクのマージまで開発者が責任持って運用しましょう」 ということになります。
過去には以下のような記事を書いていて、そこでもチラッと書いていることになります。
GitHubの使い方というより運用方法に焦点を当てていますので、GitLabや他のサービスを使用している方にも参考にしていただければと思います。
一般的にマージってどうしてる?
一般的に、プルリクのマージは「シニアエンジニアがマージ」「リポジトリ管理者が実行」といった形で行われることが多いようで、実際に私達のチームでも過去には下記のような運用をしてました。
- 開発者がプルリクを作成
- プルリクのレビュアにシニアエンジニアをアサイン
- シニアエンジニアがレビューを実施
(必要があればシニアエンジニアから開発者へ修正依頼) - シニアエンジニアがメインブランチにマージ
しかし、私たちのチームでは、プルリクを作成した開発者が最後まで責任を持つプロセスを採用しています。レビュアにプルリクを ぶん投げる のではなく、開発者自身がマージまでのプロセス全体を管理します。
現在のマージ方法
私たちの現在のマージプロセスは次のようになっています。
- 開発者がプルリクを作成
- プルリクのレビュアに他のエンジニアをアサイン
- アサインされたレビュアがレビューを実施
(必要があればレビュアから開発者へ修正依頼) - レビュアがプルリクに対してapproved
- 開発者がメインブランチへマージ
また、GitHubのブランチ保護ルールを活用して、approvedが1個以上ないとマージができないようになっています。
ブランチ保護ルールって何?
1つ以上のブランチに特定のワークフローを強制することができます。たとえば、承認レビューを要求したり、保護されたブランチにマージされるすべての pull request について状態チェックを渡したりすることができます。
とあります。
画像のように「Require approvals」を「1」で指定していることで「approvedが1個以上ないとマージができない」状態になります。
なぜこの方法に変更したのか?
以前のマージ方法では、いくつかの問題がありました。
プルリクの埋没
レビュアにプルリクが集中してしまうため、レビューの時間が確保できず(それでなくてもシニアエンジニアにはタスクが山積)レビューまでに数日経過しまうことやプルリクの修正が検知されずに埋もれることがありました。
また、プルリクが埋没して時間が経過していると、開発者自身でも「どんな実装をしたのか忘れてしまった」「他のプルリクがマージされていて、開発時の仕様と変わっている」等によってコンフリクト対応や障害発生時の修正に工数を溶かしてしまうこともありました。
お祈りマージ
大きなプルリクは、どんなに経験豊富なシニアエンジニアであっても、完全には把握できないことがあります。
そうすると、どうしても「何も起きないでくれ」と祈りながらマージすることになります。
ぶん投げ
プルリクを作成した段階で、プルリクの責任者がシニアエンジニアに移ってしまい、開発者からプルリクを 「ぶん投げ」 の状態になっていました。
やや表現は悪いですが「実装したから、問題なければマージしといてね」というお任せ状態です。
などの問題に対処するため、マージプロセスを変更しました。開発者がプルリクの作成からマージまでの全プロセスを管理することで、責任あるコード管理ができると期待して変更しました。
どんな恩恵があった?
この「開発者がマージする」プロセスによって色んな恩恵がありましたが、最大の恩恵は開発者がサービスに近づいたことだと考えています。
背景にはプルリク作成者の責任感の増加やリリースまでのプロセスを把握できる等はありますが、自分の実装がリリースされて、どのように貢献できているか今までより身近に感じることができます。
今までのように、どこかで「プルリクを任せた」になってしまうとリリースまでに開発者の手から離れるタイミングが発生して、場合によっては戻ってこないままリリースされることもありました。
それがマージしてテストしてリリースされるまで管理が途切れないことでサービスとの距離感が縮まっていると考えています。
責任感の増加
マージが完了した時点でチケット完了と見なされるため、マージできないとチケットが「テスト・レビュー中」で止まってしまい完了になりません。
開発者も実装が終わっているのに進捗が停滞している状況は避けたいので、レビュアに対して積極的にレビュー催促をしてマージできるように働きかけることになります。
プロセスを把握
開発者自身がマージしているため、マージ後のテスト実施タイミングも把握しやすくなりました。
これは「マージしたけどステージング環境でのテストが漏れていた」という状況も回避できます。
どんな課題がありそう?
とはいえ、良いことばかりではありません。今後もカイゼンするべき課題はあります。
シニアエンジニアのみにレビューが集中する状況がボトルネックになっていたため、開発者間の相互レビューを取り入れてレビュアを増やしています。
そのため、各開発者のレビュー負荷が上がっていることは事実です。
開発者には実装力とともにレビュー力、特にアーキテクチャの理解や読み解く力が求められることになります。
十分なレビューがされずにapprovedが押されると、アーキテクチャに則していないコードがマージされるリスクが上がります。
さらに、レビューに慣れていない場合は、レビュー会を開催して開発者が実装の意図をレビュアに説明する必要があるため双方の時間が削られます。
ただ、エンジニアには実装以上にソースを読む能力が求められます。
レビューの負荷を課題として解決するより、どのように乗り越えたほうが開発組織全体として底上げになるのか?も考えて課題に向き合っています。
今後の展望
開発体験の向上に向けて「こういうルールだから」と思考を止めずに改善を続けています。
現在のマージプロセスが最適とは限らず、開発組織の規模や他のフローが変われば以前のマージプロセスに戻す可能性もあります。課題を理解して適切な対応を取りながら進めていくことが重要だと考えています。
今回は一般的なマージプロセスと異なるアプローチを紹介しました。私たちの組織では、現在の方法が良い方向に進んでいると感じていますが、引き続き課題に取り組み、開発者体験を向上させていきたいと思います。
We're Hiring!
最後までお読みいただきありがとうございます。
株式会社DELTAでは仲間を募集中です!!
まずはカジュアル面談からできればと思います、ぜひPittaから
またはGoogle Formから お申込みください!
Discussion