はじめに
こんにちは、D2Cエンジニアの原尻です。
私の所属するチームでは振り返り会でKPT法を用いています。先日、P(Problem)の部分でコードレビューに対しての言及が複数あったため、改めてコードレビューについて見直す会を設けました。
本記事では、チームで感じていたコードレビューに関する課題と解決のために作ったルールについて述べようと思います。
課題
現状のチームでのコードレビューの課題としては以下があげられました。
レビュワー選定の課題
- レビュワーの選定に迷うことがある。
レビュー速度の課題
- レビューが詰まり作業が遅れた。
- レビューがどのタイミングで完了するかわからないため、レビュイーの心理的負担がある。
- 個々の判断に委ねられているためレビューの速度にばらつきがある。
その他
- レビューによりコードの保証ができているか心配。
- PM, PLなど責任者がレビューをすることが一般的だが、体制が整っていない。
- レビュワーが不足している。
- レビューの粒度をどうすればよいか迷うことがある。
弊チームは比較的若い年次で構成される7人のチームです。元々決まったルールはなくPR(プルリク)ごとになんとなく過去に経験したことのあるメンバーをレビュワーに選定していました。そのため、レビュワー選定する際の心理的障壁やレビュワーごとの速度や質のばらつきが課題にあげられました。
これらの課題を解決するためにチームで話し合い、コードレビューに対して新たなルールを設けることとしました。
レビューの観点
レビューの観点に関することは経験やスキルに大きく依存するため、今回はスコープ外とし深堀はしないこととしました。
今回は心にとめておくこととして、Googleのコードレビュー開発者ガイドを参考にレビューをする際やコードを書く際に気をつける観点を共有しました。別途定期的に振り返り必要に応じてブラッシュアップしていく予定です。
原則的な考え方
- 存在するのは「より良いコード」のみで、「完璧な」コードは存在しない。
- CL（Change List）が完璧でなくても、全体としてシステムの健康状態を向上できるのであれば、承認して進めるべき。
設計
- モジュール分割が十分細かい、関数の行数が多過ぎないなど、コードを読んですぐに理解し使用できるか。また別の開発者が簡単にコードを使用したり改修できるか。
機能性
- 開発者だけではなく、ユーザーにとっても適切であるコードになっているか。
- レビュー依頼前にテストと動作確認を済ませ、意図した通りに動いたか、テスト環境がない場合は、テスト環境構築の計画を立てたか。
コメント
- なぜ他のやり方ではなくこうなっているのかなど、コードに対する自分の考えを残しているか。
- 何をしているコードなのかなど、コードを読めば理解できることはコメントには残していないか。
- レビュワーは意図がわからないコードについては躊躇せずに質問し、コメントを追加するよう依頼する。
命名
- 何であるか／何をするかがわかるものになっているか
スタイル
- コードはスタイルガイドに準拠しているか
ドキュメント
- 必要に応じて関連するドキュメントも更新しているか
レビューのルール
最終的なルールとして新たに以下を定めることにしました。
小さなCL(Change List)
原則、1つの機能実装に対して1つのPRとする。
- タスク自体を細分化し、子課題に対してPRを作成するようにする。
- 子課題のスコープについては、必要に応じて取り組む前に都度相談する。
- 1つの機能の最小単位は、PRが通った時にビルド・リリースできること。
- 機能追加とリファクタリングは別のPRで行う。
PRは冗長化せずレビュワーが読みやすい大きさにするという基本的な原則を共有した上で、経験によるものが大きいためその都度指摘し徐々に学ぶという結論に至りました。
レビュワー選定
- チームメンバーのうち二人を固定レビュワーにし両名が承認したら先に進む。
- 稼働などの問題で難しければ、固定レビュワーが他のメンバーを追加する。
- デフォルトレビュワーがPRを作成した場合、片方のレビュワーと星取表が○以上のメンバーをレビュワーにする。
レビュワー選定に関して、レビュワーを固定しないことによって把握漏れが生じるリスクや共有の手間を考えると、固定にする方が以前よりむしろ負担が減るのではないかという意見が出ました。また、固定にすることでレビュワー選定の迷いやレビュワー間のばらつきを解決できるため一度レビュワーを二名に固定にし試すことにしました。
また、レビュワー選定に困る原因としてチームメンバーの技術スタックを把握しきれていないという点があげられたため可視化できるように星取表を作成しました。
★:エース
◎:得意
○:一人でできる
△:助けがあってできる
↑:習得したい
選択なし:できない
として、縦軸にチームで使っているスキル、横軸にメンバーの名前を書いて表を作成しました。
テンプレート
レビュワー側の負担を減らすため、以下を記載項目（太字は必須項目）として、Bitbucketのデフォルトテンプレートに設定することとしました。
- 作業背景と理由
- Jiraに記載済であれば不要。「〇〇の依頼で〇〇を解決するため」のように完結な記述でOK
- 作業内容
- レビュー期限
- 急ぎの時のみその旨を記述
- 検証結果（チェックリスト）
- dev環境での検証を実施したか
- prod環境でcdk diffの確認をしたか
- フォーマットをかけたか
- ハイライト
- 不安な箇所、特に注意してレビューしてほしい部分があれば記載する
実際のテンプレートは以下のようになりました。
期限
- 原則、1営業日以内にレビューを終えるようにする
- 例)11/21の14:00にPRを出したら、11/22の18:00までにレビューを完了する
- 2回以上往復する内容は、コメントでの議論はやめ、口頭議論やモブプロで解消する
- 議論や決定内容はPRに残すようにする
レビュー詰まりを防ぐために具体的な期限を設定しました。
期限が厳しい場合は原因を考えそちらの方を解決するアプローチをとることとしました。
やってみて
実際にやってみた感想をまとめました。
-
レビュワー選定
- レビュワー固定にしたことで、レビュワー選定に迷わず快適になった。
- ルールができたことで曖昧になっていたレビュワーの判断がしやすくなった。
- (レビュイー側)レビューの負担がなくなり作業に集中できるようになった。
- (レビュワー側)負担が急に増えたということは感じない。
-
デフォルトテンプレート
- リポジトリごとに最適化するなど改良の余地がありそう。
- （レビュワー側）テンプレートがあったほうがレビュイーが必要なことを書いてくれるのでレビューがしやすい
-
期限
- レビュー詰まりが全くなくなりすぐに進むようになった。
- リリースの頻度が上がった気がする、、？
レビュワー選定と期限の施策についてはレビュワーレビュイー共に高感触でした。一方でデフォルトテンプレートの導入に関しては、リポジトリごとのルールを改めて見直しテンプレートをブラッシュアップする必要がありそうです。
まとめ
今回はコードレビューの観点やコードの質をスコープから除き、弊チームで検討したコードレビューの運用方法について記述しました。
コードレビューといった運用面では、ベストプラクティスをそのまま適用するのではなく、チームにあった方法を都度見直していく必要があります。今回は、このチームでのベストな運用方法は何かを話し合う中で、チームでの課題やメンバー各々の課題の可視化と共有にもつながりました。課題の把握と改善を繰り返し今後もよりよいレビュー体制が構築していきたいと思います。
また、レビューにとどまらず他の課題の解決にも取り組むことでよりよいチームとプロダクトの保守運用ができればと思います。
弊チームの一例の紹介によって、少しでも何かのお役に立てれば光栄です。
最後までお読みいただきありがとうございました。
参考文献
株式会社D2C d2c.co.jp のテックブログです。 D2Cは、NTTドコモと電通などの共同出資により設立されたデジタルマーケティング企業です。 ドコモの膨大なデータを活用した最適化を行える広告配信システムの開発をしています。
Discussion