コードレビュー文化のある組織づくり
ソフトウェアエンジニアの 渋谷 です。
コードレビューを実際のプロジェクトでどのように活用しているかを紹介します。
今回は私が入社当初からお世話になっているプロジェクトマネージャー（PM）2名に協力いただき、特徴的なプロジェクトについてインタビューしました。
コードレビューの目的、方法などについては多くの記事で紹介されているため、この記事では省略いたします。
コードレビューについて知りたい方は私が参考にしている代表的な記事を2つ紹介するので、そちらを見てください。
斉藤さんにインタビュー
斉藤 さんからは2つのプロジェクトを紹介します。
- WebAPI、Webクライアント保守プロジェクト(人数: 2人)
- モバイルアプリリニューアルプロジェクト(人数: 5人)
WebAPI、Webクライアント保守プロジェクト(人数: 2人)
Q. プロジェクトについて教えてください。
WebAPIとWebクライアントを保守するプロジェクトです。
体制はProject Manager(PM)とPMを引き継ぐ2人のプロジェクトでWebAPIとWebクライアントの保守をしています。
最終的には引き継ぎ後、1人でプロジェクトを進行できるようになってもらうことを目的としています。
Q. コードレビューをどのように取り入れていますか？
開発序盤の頃はペアプロのように一つの画面を 見ながら丁寧に説明することもありましたが、基本は作成されたPull Request(PR)をレビューしています。
PRのレビュー内容は主に動作確認と適切に処理が書かれているか確認しています。
Q. 「適切に処理が書かれているか」の部分について詳しく教えてください。コードレビューはどこまでチェックしていますか？
大まかには仕様を十分に満たしているかどうか、バグの原因になりそうなコードになっていないかをみています。
具体的には、バリデーションやエラーハンドリングなどを中心に見ており、変数の命名やファイルの分け方など、より詳細な部分については開発者に任せています。
理由としては、開発者の思想次第で良いと思うコードは異なるため、明らかにバグの原因になりそう、デバッグがしづらくなりそうと判断しなければ、特に指摘することはありません。
どうしても気になるときは、修正するかどうかは任意であることを明らかにした上で伝えるようにしています。
モバイルアプリリニューアルプロジェクト(人数: 5人)
Q. プロジェクトについて教えてください。
モバイルアプリのリニューアルをするプロジェクトです。
体制はPM1人、Project Leader(PL)1人、Programmer(PG)3人の合計5人で開発をしています。
Q. コードレビューはどのように行なっていますか？
コードレビューの話の前にまず、開発メンバーの認識合わせを行い、
進捗確認の方法、会議の種類、アーキテクチャの方針を事前に決めることで実装の後戻りを減らしています。
ここから、コードレビューの話になります。
GitHubを利用しているため、開発者はPRを作成し、それをPMとPLがレビューしています。
ただ、すべてのPRをレビューすることは負担が大きいため、PMはコードレビューをしないと決めました。
PMは動作確認のみ行い、PLがコードレビューを行うことで役割を分散させました。
他にもコードレビューの負担を下げる仕組みを入れています。
GitHub Actionsを使い、PR作成時にLint、Formatterのチェックを入れ、このチェックが通ることで初めて人がレビューすることになります。
つまり、PRがマージされるためには、Lint、Formatterのチェック、PMの動作確認、PLのコードレビューの3つが必要になります。
Q. コードレビューに何か課題はありますか？
レビューの手が回らず、レビュー待ちの状態になることが続いたことがあります。同時に4つにPRが作成されるとどうしても手が回らなくなってしまいます。
もう一つは、あるタスクに依存している別タスクがあり、そのPRがマージされないと他のタスクに着手ができないことがありました。できるだけタスクは切り分けるようにしているのですが、開発終盤に発生してしまいます。
これらを時間かけずに解決できる方法は見つけられていません。しかし品質担保のためには、時間を使ってレビューすることが泥臭いけど良い方法かな、と考えて進めています。
江口さんにインタビュー
江口 さんからは一つのプロジェクトを紹介します。
Webクライアント開発プロジェクト(人 数: 7人)
Q. プロジェクトについて教えてください。
6つのWebクライアントの開発、保守プロジェクトです。
レビュー体制が整えられていないプロジェクトの引き継ぎからレビュー体制を整えた事例になります。
チーム人数は7人で、構成はPM1.5人、PL1.5人、PG4人の構成になっています。
(江口さんはPM、 PLを兼任しているため、PM0.5、PL0.5で加算しています。)
Q. コードレビューはどのように行っていますか？
まず、チームで決めたルールがあります。
Gitブランチ運用や、コーディング、セキュリティなどをPRチェック表として定めており、すべてのチェック項目を満たさないとPRを作成できないルールになっています。
ルールを守った上でPRを作成するので、レビューでの指摘は少ないと思います。
次にPRのレビューをします。
レビュワーの細かな動作確認を任意とし、「要件を満たしているか？」の動作確認はテストフェーズでの責任と明確に分けてます。コードレビューではあくまで「要件・仕様を満たしているのは大前提で、プログラム的に行儀悪いことをしていないか？パフォーマンスに影響ある書き方がされていないか？」などに焦点を当てています。
PRのマージ条件はチームの過半数（6人中3人以上）がapproveしたらマージ可能としています。
Q. ルールはどのように決めましたか？
引き継いだ直後はレビュー体制が整っていなかったため、フレームワークのバージョンアップの機会にレビュー体制を整え始めました。
まずは、基本的なコーディング規約を当初は私1人で作成しました。
ただ、実際に開発が続いていくと不都合がでてきます。その都度話し合い、納得した上でルールの変更、追加を行っています。
Q. どのように話し合っていますか？
ルールの変更、追加を行いたい場合は匿名で多数決を行い、過半数が賛成することで採用されます。
コードの書き方は個人の好みで左右されやすく正解はないため、チームで納得することが重要だと思いこの方式にしています。
だいたい、1年ほどでルールが固まっていったと思います。
Q. PR作成からレビューされるまでの時間はどの程度でしょうか？
だいたい24時間以内にはレビューされています。
毎日ミーティングを行っており、進行が滞っていれば都度話し合っているので、PRのレビュー待ちが残り続けることはないと思います。
Q. コードレビューで何か課題はありますか？
特に無いです。
チームで合意した内容でレビューを進めていますし、メンバー全員が自主的に動くためレビューも滞ったことはありません。
何か課題があれば、すぐに話し合うことで解決しているので、課題を放置していないだけかもしれないですね。
まとめ
今回はチーム構成が2人〜7人のプロジェクトの事例を紹介しました。
チーム人数に応じてコードレビューの運用の仕方が異なっていることがわかると思います。
お二人に共通していることは、事前にルールを決めているところです。
コードレビューではチームの規模が大きくなるほど開発者の思想が ぶつかりやすくなります。事前にルールを決め、チーム全体の認識を合わせることで円滑に開発を進めていました。
プロジェクトの状況に合わせてコードレビューを運用していきたいですね。
