Pull Request のレビュー効率をあげるためにやっておきたいこと

サムネイル

はじめに

こんにちは。

プラットフォーム事業本部第一開発部アカウントサービスグループ所属の小山由人と申します。

2021 年 7月にチームへ参加し、バックエンドエンジニアとして アプリケーションの機能開発や保守運用をしています。

この記事では 2022 年 7 月ごろから携わっているアカウント基盤の Java から Go へリライトする際のレビュー効率化を図る取り組みについてお話します。

近年、私が所属するプラットフォーム事業部ではシステム移行作業が各チームで行われてきました。

私は主にアカウントを管理するアプリケーションを Java から Go にリライトするというプロジェクトに携わってきました。

移行対象のアプリケーションにはリライト対象となる API とバッチは合計 100 本近くあり、仕事をなるべく早く効率的に進める必要がありました。

無事にリライトが一段落ついたので 1 年間をふりかえり、どのような工夫を行ってきたのかも併せて紹介していきます。

プロジェクトの概要

プラットフォーム事業本部では、まずエンジニアが仕事を進めるにあたり、どのように開発を進めていくかをまとめた「Design Doc」というドキュメントを作成します。Design Doc についてはこちらを読んでみてください。

そのうえでリライトプロジェクトを始めるにあたり、次の Goal と Non-Goal を設定し、これらを基にリライトを進めてきました。

Goal

アプリケーションを Java から Go へリライトする。

Non-Goal

動作の互換性を担保した状態にするため以下には変更を加えない。

  • 提供するインターフェース(API)
  • 処理シーケンス
  • DB のテーブル構造

リライト対象の API とバッチは合計 100 本ほどあり、リーダー 1 名と私を含めた開発者 4 ~ 5 名の体制で 2022 年 7 月から 約 1 年間かけて行いました。

チームの生産性可視化の取り組み

プロジェクトを進めるにあたり、チームの生産性可視化のツールとして「Findy Team+」 を使っています。

下記のように、Pull Request のオープンからマージまでの平均時間と Pull Request 数のデータをリードタイムサマリとして可視化してくれます。

またサイクルタイム分析機能も活用しています。

サイクルタイム分析について

サイクルタイム分析では、開発リードタイムを 4 つのプロセスに分けて可視化してくれます。

各プロセスの時間が短いほど 1 つの Pull Request の消化スピードが早いことを意味します。

コミットからオープンまでの平均時間は、タスク開始から Pull Request をオープンにするまでの時間ではないため、あくまでも「参考値」として見ています。

定期的にチームでふりかえりの機会を設け、サイクルタイム分析を活用し、開発プロセス内のボトルネックを可視化しています。

今回、この記事を書くにあたっても「Findy Team+」 を使ってみましたが、時系列データがみれることからプロジェクトのふりかえりにも使える便利なサービスだなとしみじみ思っています。

生産性向上の実績

プロジェクトで開発フェーズが活発であった2022 年 7 月 1 日から 2023 年 7 月 31 日 までをサイクルタイムの「平均値」で表してみました。

  • コミットからオープンまでの平均時間: 11.2h
  • オープンからレビューまでの平均時間: 1.9h
  • レビューからアプルーブまでの平均時間: 9.7h
  • アプルーブからマージまでの平均時間: 1.2h

各プロセスの平均時間が短ければ短いほどよく、「Findy Team+」側の基準に沿ってランク付けされます。

全指標が High 、 Elite であり、リライトプロジェクト全体において高い生産性を維持できたといえます。

生産性向上のための取り組み

なぜ、今回のプロジェクトでは生産性を高くできたのかをふりかえってみます。

サイクルタイム分析における 4 つのプロセスごとにチームでの工夫をまとめていきます。

コミットからオープンまでの時間短縮の取り組み

タスクを開始してから Pull Request を作るプロセスの平均時間を向上するための取り組みについて説明します。

迅速な環境構築

アプリケーションからデータベースまでを迅速に起動できる環境を整えました。

docker compose up するだけで、アプリケーションからデータベースなど各種必要なミドルウェアすべてを起動できるようにしています。

Docker さえあればすぐに API を動かせる状態になっています。

またコンテナ経由で lint や gofumpt 、テストを実行するように Makefile も準備しています。

  .DEFAULT_GOAL := help
  .PHONY: help
  help: ## help
      @grep -E '^[%/a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | awk 'BEGIN {FS = ":.*?## "}; {printf "  \033[36m%-22s\033[0m %s\n", $$1, $$2}'
      @echo ''
  
  .PHONY: test
  test: ## run unit test
      docker compose exec app go test -p 1 -shuffle on `go list ./... | grep -v ./test/integration`
  
  .PHONY: integration-test
  integration-test: ## run integration test
      docker compose exec app go test -p 1 -shuffle on ./test/integration/...
  
  .PHONY: lint
  lint: ## run lint
      docker compose exec app golangci-lint run
  
  .PHONY: fmt
  fmt: ## format
      docker compose exec app gofumpt -l -w .

環境構築に加えてデバッグが容易にできるようなしくみも導入しています。

もともと Java を扱っているチームメンバーが多く、大半は GoLand を使用していたので、テスト実行時に必要な環境変数が実行構成に自動で適用されるようなプラグインと実行構成のコード化も導入しています。

EnvFile

実行 / デバッグ構成を共有する方法

新しく参加した開発者もすばやく開発に取り組めます。

Pull Request 1 つあたりの差分を小さくする

それから 1 つの Pull Request のサイズを小さくするようにタスクを切り分けしました。

API を実装する場合、当初は下記のようにタスクを分けていました。

  • ユーザー検索 API
  • ユーザー登録 API
  • ユーザー更新 API

このような分け方では 3 つの API で共通的に使うだろう部品を作らなければ実装できないため並行してタスクを進めにくいです。

また、タスクの粒度が粗いため 1 つの Pull Request あたりの変更行数が多くなってしまいます。

テンポよく実装を進めるために、これらの課題を解決する必要がありました。

今回のリライトプロジェクトにおいてアプリケーションコードのアーキテクチャはオニオンアーキテクチャを採用しています。

ですので、 Entity や Value Object 、 Repository というように必要な部品ごとにタスクを分けることにしました。

たとえばユーザー検索 API であれば下記のように分けました。

  • ユーザー ID の Value Object の実装
  • 性別 の Value Object の実装
  • ユーザー Entity の実装
  • ユーザー Repository の実装
  • ユーザー Repository に対する Infra 層の実装
  • ユーザー検索用 UseCase の実装
  • ユーザー検索用 UI 層の実装

このように 1 つのチケットに必要な実装の範囲をなるだけ小さくすることで着手開始からレビューに出すまでにかかる時間を短くできました。

実装範囲が小さくなることで Pull Request あたりの差分も少なくなり、レビューに要する時間も削減でき、サクサクと開発作業を進められました。

また、 Entity や Value Object などはほかの UseCase でも利用することが多いので優先して実装しました。

このように進めることで API の実装を並列に進めることも可能になりました。

「Findy Team+」でもタスクの粒度を変更した時期のリードタイムと Pull Request 作成数を確認すると、リードタイムが減少して Pull Request 作成数が増加しており、生産性向上の傾向がみてとれます。

シーケンス図の整理とテストコード拡充

Go でのリライトを始める前に準備も実施しました。

その例として、シーケンスの整理と Java でのテストコード拡充があります。

全 API の処理シーケンスを Mermaid を使って管理しています。

レビュー時に実装とシーケンスを比較しながら確認したり、シーケンス図から必要となるテストコードを考えることができたりしてとても役に立ちました。

また、 Java 側においてテストコードが不足していると感じる場合は Go 側の実装着手前に Java 側のソースコードに対するテストコードを拡充しました。

処理の詳細を理解するのに役立ちますし、既存のテストコードと鏡合わせになるように Go 側のテストコードを作ることで既存仕様が踏襲できていることも判断しやすかったです。

今回のようなリライトプロジェクトにおいてはリライト元のリファクタが非常に重要だということを学びました。

オープンからレビューまでの時間短縮の取り組み

CI/CD の整理

私たちのチームでは CI/CD パイプラインの構築に GitHub Actions を使用しています。

リポジトリ作成時に最低限の CI は整えており、 pull_request イベントをトリガーにして golang-ci-lint を使った Lint の自動実行、テストを実行してカバレッジレポートの作成とアップロードをしています。

カバレッジレポートは分岐網羅が満たせていない箇所に気付きやすく重宝しています。

CI/CD の成功率や実行速度の可視化

DMM ではアプリケーションの運用監視ツールに Datadog を使っているのですが、 Datadog には GitHub と連携して CI ごとの実行時間や失敗率を可視化する CI Visibility という機能があります。

このように CI の実行時間を可視化できます。

また、失敗したワークフローが一定期間内に何件あるかや、失敗したワークフローを一覧にすることもできます。

定期的に開発メンバーで集まり、実行時間の削減や失敗率の低減のために何か工夫できることはないか議論しています。

レビューからアプルーブまでの時間短縮の取り組み

ファーストレビューの速度を上げる工夫

Pull Request が Open になってからファーストレビューの時間を短縮するために下記のような工夫をしています。

  • Auto-Assign を使った Reviewer の自動アサイン
  • Scheduled reminder を使って Slack 通知

自動アサインについては GitHub 公式の機能もありますが、より柔軟にアサインができる Auto-Assign を利用しています。

通知は GitHub の Scheduled reminder を使っており、アサイン時だけでなくスレッドへの返信やメンションも Slack へ通知されるようにしています。

Reviewer に優しい Pull Request を作る

レビューを円滑に進めるために Author は Reviewer がなるだけレビューしやすい状態にしておくように心がけています。

Pull Request を見れば何を何に変更したのかはすぐに分かります。

では、なぜその変更が必要だったのかという点については実装を見るだけですぐに分かるものもあればそうでないものもあります。

変更するに至った文脈を Pull Request の説明に書くのはもちろんですが、インラインコメントで実装部分に補足としてコメントしておくことで Reviewer の負担を軽減するようにしています。

ほかにも、複数の実装方法が可能な場合になぜ他の選択肢を採用しなかったのかといったような想定できる質問に先回りして回答しておくことも大事です。

Reviewer が推測するために使う時間を減らせます。

Reviewer ファーストな Pull Request を作ることで Reviewer も楽、 Author も早くタスクが片付いてうれしいという Win-Win な状況を作っています。

同期レビューにボイスチャットツールを活用

ただ、それでも Reviewer 側に疑問点があったり、 Author が気付いていない考慮漏れが見つかることはあります。

そんなときにはボイスチャットツールの Discord が非常に役立ちました。

私たちのチームではコミュニケーションツールの 1 つとして Discord を利用しています。

複数の部屋を用意しておき、基本的に開発メンバーはどこかのチャンネルに常駐しています。

何か聞きたいことがある場合には任意の部屋に集まって同期的にレビューをしています。

チャンネルも多いので 2 ~ 3 人で集まりやすく、レビューだけでなく金曜日の夜には雑談も楽しんでいます。

なんだかんだで話しながらだと、思わず不具合に気付けたり Reviewer がどのようなところが分かりづらくて負担になっているのかなどが伝わり、次のレビューに向けた参考にもなっていました。

Zoom や Google Meet でも同期レビューは可能ですが、Discord の仕様ならではの声のかけやすさがとても良かったです。

Reviewer としての心構え

チームの文化として、レビューをしない人はレビューされないと考えて行動するというものがあります。

チームメンバー全員が基本的には自分のタスクよりもレビューを優先しています。

なによりもレビューが早く終われば結果的には、自分自身のタスクをレビューしてもらえる時間も増えるので実利を伴うすばらしいマインドセットだと思っています。

相互にすばやくレビューすることでチーム全体の生産性が高く保たれています。

アプルーブからマージまでの時間短縮の取り組み

アプルーブされたら Slack へ自動通知されるのですぐにマージするようにしています。

それから、アプルーブからマージまでの平均時間向上の取り組みに限りませんが、Pull Request が Open な状態で休日をまたがないようにしています。

土日 2 日間を挟むだけでも、週明けにこの Pull Request なんだっけとなるくらいに記憶がリセットされてしまいますからね。

月曜日のフレッシュな脳のエネルギーを無駄遣いしないようにも仕事は金曜日で一区切りさせるようにしています。

課題

もちろん課題もあります。

たとえば、データベースと連携するようなテストコードを書くときです。

Java のときは Database Rider というテスト用ライブラリを使ってテストデータのセットアップや、期待値の検証を YAML ファイルを利用して行うことができます。

https://github.com/database-rider/database-rider

Go でも似たようなしくみをもつ testfixtures というライブラリを使っています。

しかしながら、このライブラリには期待値を検証する方法がないため、自前でテスト実行後のテーブルのレコードの検証を書かなければいけず、なかなか工数がかかりました。

期待値のアサーションができるしくみやライブラリの作成を検討しています。

また、バッチの実装時にも Java のときに利用してた Spring Batch のしくみをなるだけ踏襲しなければならず、 Go での実装が複雑化してしまっているという問題があります。

バッチ実装当時のサイクルタイムも Pull Request の作成数もやや悪化していたということが分かります。

バッチは実装の本数も少なかったのですが、今後の開発のことを考えて「バッチを容易に実装できるしくみを作りたいね」とチームでは話しています。

まとめ

私たちのチームでは、アプリケーションを Java から Go にリライトするプロジェクトを進めてきました。

チームの生産性を可視化し、開発フェーズにおけるサイクルタイム分析やタスクサイズの調整、レビュー工程の効率化など多くの取り組みを行っています。

これらの取り組みはプロジェクト開始当初からすべて高い水準で実践できていたわけではありません。

私たちのチームではスクラムで開発を進めており、スプリントごとにふりかえりを行います。

良かったことや課題感を共有し、次のスプリントへの Try を決めていました。

また、 Discord に夕方なんとなく集まって雑談していたのも、今考えるとミニふりかえりとしての役割を果たしていたのかなと思っています。

頻繁にふりかえり活動を行い、 1 人ひとりがより仕事を楽しく進められるようにメンバーが改善活動を行ってきた結果、高い生産性を保つことができています。

宣伝

私たちのチームでは 4,101 万人(2023年2月時点)の DMM のアカウントを管理するサービスを運用しています。

Go を使った開発やクラウドネイティブアーキテクチャの構築する経験や国内トップレベルのトラフィックが求められる環境でのシステム設計と運用の知識はキャリアにおいて大きな武器になります。

今回の記事で紹介したような、エンジニアが楽に働けるような環境の整備も継続的に行っています。

開発者体験向上の取り組みに興味のあるエンジニアにとってもたくさんの成長機会があります。

こんなに刺激的な環境はなかなかないのではと思っており、毎日楽しく働いています。

ご興味をお持ちいただけましたらぜひご応募ください。

DMMの求人情報