こんにちは。カバー株式会社のKです。普段はタレントが配信に使用する「ホロライブアプリ」の開発に携わっています。
ホロライブアプリはUnityで開発しています。詳しくはこちら↓の弊社技術ブログをご覧ください。
私はアプリ開発チームのリーダーとして、開発チームの生産性向上のための取り組みをいくつか行ってきました。今回は、弊チームで導入したコードレビューについて説明します。
課題
弊チームは当初1～3人ぐらいで開発・運用しており、新機能の実装や既存機能の修正は各自でなんとなく行っている状態でした。プルリクもなく、お互いのコードを見るのは開発・修正に必要な時のみでした。今から考えると雑ですが、人数が少なかったのもあり、たまにバグを出しつつも何とか成り立っていました。
しかし、開発メンバーの人数が増え、ソースコードを触る人数も増えるにつれ、既存の体制だと開発体験が悪い部分が目立ってきました。コードの書き方が揃っていない部分も目に付くようになりました。増えたメンバーの開発力と時間を十分に活用できていない肌感もありました。
また、チーム内でお互いのコードを読んで議論する機会が少ないままでした。コードを評価される機会がないため、より良い書き方を学ぶ機会が乏しかったです。「良いコードとは何か」ということについて、メンバー間での共通認識が育たない環境になってしまっていました。
そこで、これらの課題への対策として、コードレビューを導入することにしました。
Googleのコードレビュー
コードレビューを導入するうえで参考にしたのが、「Googleのソフトウェアエンジニアリング」という書籍です。他にも他社の事例を検索するなどして調査しましたが、結果的には弊チームのコードレビューはほぼこの書籍で得た知識でルール化されています。
この書籍の「9章：コードレビュー」にて、Google社内のコードレビューについての考え方やルールが述べられています。弊チームに取り入れるうえで注意した点は以下です。
レビュアーを「門番」にしない
コードレビューのレビュアーをテックリードなどの責任者のみが担当し、あたかもコードがマージされる前の「門番」のように立ちはだかっているという会社やチームもあります。しかし、この方法ではレビュアーにレビューの負担が集中してしまい、レビュアーが忙しくなるとそこがボトルネックになって開発プロセス全体も遅くなってしまいます。なので、誰かを「門番」にするのではなく、チームメンバー同士でレビューしあって柔軟にスケール出来る仕組みにするべきです。
レビュアーの役割を分ける
レビューのプロセスに携わる人の役割を分けます。上記の書籍では、「作者」と「レビュアー」という役割を置いており、レビュアーの役割はさらに、「動作チェック」、「承認」、「リーダビリティ」 にさらに分けられます。レビュアーの役割を分けて分担可能にすることで、コードレビュープロセスの柔軟性が高まります。弊チームではこの役割分担を簡略化して、「提出者」、「レビュアー」、「オーナー」に分けました。
変更内容は適切に切り分け、大きくなりすぎないようにする
変更内容が大きいとレビューにかかる負担も大きくなります。一回のレビューにかかる負担と時間が大きいと、レビュー作業が億劫に感じられるようになりますし、レビュアーの拘束時間が増えて他のタスクに支障が出てしまいます。変更内容によっては難しいこともありますが、大きい変更を行うときはできるだけ課題を切り分けて独立したプルリクとして提出することが望ましいです。
完璧は求めない
コードレビューをしていると、ついつい理想のコードを追求して細かい書き方についても訂正したくなる時があります。しかし、コードの品質を必要以上に上げるのは提出者にもレビュアーにも負担がかかります。それでレビューが面倒なってしまっては本末転倒なので、完璧を求めるのではなく、低負担で持続可能な仕組みにすることを意識しました。
できるだけ自動化する
レビューはどうしても時間を使う作業です。レビュアーは自分のコードを書いたり他の作業をするための大切な時間を割いてレビューをしているので、人間がやらなくて済む部分に関しては自動化し、手間を減らすべきです。弊チームでは、GitHub Actionsによる提出時の自動テストや、GitHubとSlack連携でレビュー担当へのメンションなどが自動で飛ぶようにしています。
導入したレビュープロセスの概要
コードレビューを導入したことによって誰かの負担が増えすぎたり、ルールが厳しすぎてメンバーがしんどくなるのは避けたいところでした。できるだけ無理のない最小限のルールでコードレビューを習慣化できることを目指してルールを作りました。
以下が弊チームで導入したルールの概要です。
目的
- ドキュメントを残す
- 凡ミスによるバグを防止
- コーディングに関する健全な議論の機会が提供される
- コードのリーダビリティ向上
方法
プロセス内のアクター（役割）は以下の３つ
- 提出者
- プロジェクトに変更を加えて、pull requestを提出する人
- レビュアー
- コードの動作確認とレビューを行う人
- オーナー
- そのリポジトリの責任者。マージを行う
手順は以下
- 新機能を追加するときや既存機能に変更を加える際は、feature/[feature名] でブランチを作り、developにGitHub上でプルリクエストを送る。
- pull requestの提出者は、一人のチームメンバーにレビューを依頼する。依頼された人は速やかににレビューする。
- レビュアーはプルリクに記述された通りにコードの動作確認を行う。
- 「レビュー基準」を元にチェックする。良さそうであればプルリクのコメントに「レビュー完了しました」「LGTMです」などコメントする。
- 提出者はレビュー完了を受けて、コードのオーナーにマージ依頼をする。
- オーナーはプルリクの内容を確認して、マージしても問題なさそうであればマージする。
hotfixなど、緊急性が高く変更量が少ない変更については必ずしもレビューを通さなくてよいことにしています。
コード規則の基準についてはここでは掲載しませんが、概ねMicrosoft公式のC#のコード規則に沿って行なっています。
導入後の所感
レビューが行われるようになったことで、コードの改善についての議論がPull requestのコメントを通して行われるようになりました。これによってメンバーが知らなかったC#の書き方を知るきっかけが生まれ、メンバーのスキル向上に繋がっていると感じます。また、クラスの分け方などコードの設計についての議論も生まれるようになりました。
実装者が凡ミスをしていた部分や見落としていた部分をレビュアーがマージ前に指摘することで、バグが組み込まれることを事前に防ぐ役割も果たせています。
コードレビューがチームにかける負担は最小限に抑えられたと感じています。特にレビュアーとオーナーを分けたことでレビューの負担をメンバーに分散できたことが大きいです。レビュアーは変更部分のコードが読めるメンバーであれば誰でも担うことができるので、その時手が空いているメンバーに柔軟に回すことができます。各メンバーのタスクの合間にコードレビューの仕事がうまく収まってくれています。
最小限のルールによって、コードレビューを持続可能な形で根付かせることができたと感じています。今後はコードの静的解析の強化やテストの拡充などにより、負担の軽さを保ちつつレビュープロセスを改善していきたいです。
まとめ
弊チームのコードレビュー導入について説明しました。最小限の負担でコードレビューのメリットを享受できる体制が作れたので、一旦満足しています。コードレビューの導入に興味のある方にとって参考になれば幸いです。
ありがとうございました。