伝えたいこと
- 大事なテーマを決めて、全体に共有しようぜ
- どんなアーキテクチャも、レビューも、チーム全員の認識が揃っていないと効果が薄いぜ
どんな人にこの記事を読んで欲しいか
- コードレビューに時間がかかる
- コードレビューの効率化に悩んでいる
- コードレビューのやり方に自信が持てず、何か参考になる事例を知りたい
コードレビューには大量の工数がかかる
僕らの会社(PrAha Inc.)では全てのプルリクエスト(以降PR)に対してレビューを行ない、レビュワー2名、もしくはテックリード1名にapproveされない限りマージしません。修正点があれば指摘して、直して、再確認して、merge。
来る日も来る日も、確認、指摘、修正、再確認、merge。
体感ですが、僕たちは業務時間の1~2割をコードレビューに関連する作業に費やしているように感じました。コードレビューを改善できれば、業務効率が大きく改善するはず。そう考えて、コードレビューの文化を見直す事にしました。
コードレビューを特定の個人に返す意味とは?
- 何を書くか考え
- どう伝えるか考え
- 書いて
- 相手に伝わらなかった場合は1~3を何度も往復する
これだけ丁寧なプロセスを経るにも関わらず、このやり取りを見るのはレビュワーと、その指摘を受けた当事者だけです。他の人がコードレビューで指摘された内容に全員が目を通しているチームなんて、少ないのではないでしょうか。つまり一般的なコードレビューは自分の大切な時間をたった一人の教育に捧げるマンツーマンレッスン、という事になります。非常に効率の悪い時間の使い方ですよね。
かつ、コードレビューが当事者間で完結すると、様々なリスクを抱えることになります。
- 別のメンバーや新規参画者が同じミスを繰り返す
- 何度も同じことを指摘していると疲れるので、レビュワーが指摘しなくなる
- 指摘する人が組織を去ると知見も失われ、同じミスが繰り返されるようになる
直接やり取りをしている人だけが、その場限りで学び、その知見が他の人に伝わらない状態。コードレビューがその場限りの使い捨てになっている状態です。コードレビューを効率化したければ、まずは当事者間でコードレビューが完結する状態を脱却し、知見をチーム全体に行き渡らせなければいけません。
とはいえ全てのコードレビューをチーム全体で共有するのではコスパが悪いので、何かテーマを絞って集中的に学習したいものです。
コードレビューの大半は「どこに書くか(WHERE)」を指摘している
日々多大な時間をコードレビューに費やす中で「一体、僕らは何に対してそこまで時間をかけて指摘し合っているんだろう」と不思議に思い調べたところ、弊社内のコードレビューの指摘点は以下のように分類できました。
(WHAT:コードで何を実現したいのか)
- 例:このボタンを押した時に動かないよ!
(HOW:どうやってコードで実現するか)
- 例:Promise.allしたほうがいいよ!
- 例:forEachの中ではawaitできないよ!
(WHERE:どこにコードを書くか)
- 例:これはRepository層に書くべきじゃないよ!
- 例:これは別のClassとして切り出したほうがいいよ!
- 例:これはPubSubに任せたほうがいいよ!
- 例:これはマイクロサービスとしてCloud Functionsに切り分けたほうがいいよ!
- 例:これはクッキーに持たせるべき情報じゃないよ!
- 例:これは環境変数に切り分けておいたほうがいいよ!
- 例:ここはドメイン層だからValueObjectとして切り分けておいたほうがいいよ!
あくまで私見ですが、
WHATの指摘が多い=仕様理解・意思伝達に齟齬が多いなど、チームの業務遂行能力に問題が潜んでいる
HOWの指摘が多い=コードを書き慣れていないなど、エンジニアの基礎的な開発能力に問題が潜んでいる
こんな風に感じており、チームの開発力が最低限のレベルを越えるとWHATやHOWを指摘する回数は減り、コードレビューの大半はWHEREに集中する傾向があるように感じます。更にレベルが上がればWHEREに関する指摘も減り、HOW FASTやらDATA MANAGEMENTやらSECURITYなど、そのビジネスにとってに重要な内容に関心が移るかもしれません。
ひとまず今の自分たちのチームはWHEREに関する指摘が非常に多い傾向があったため、これを集中テーマとして取り扱うことを考えました。
特にWHEREは意見が割れて議論が起こりやすい
WHATの指摘には「直さないと動かない」、HOWの指摘には「修正が限定的で容易」と、それぞれに指摘修正をしやすい条件が整っていますが、WHEREの場合は様々な問題が浮上します。
-
直さなくても動いてるのに・・・
- (急いで対応する必要がないため、優先度が下がる)
-
ええー、今から変更するの大変なんだけど・・・
- (テストの書き方、DIの仕方など変更箇所が増えて、工数がかかりやすい)
-
うーん、あとで問題が起きてから直せば良いのでは?・・・
- (将来的なメリットをもたらすが、直近のメリットを実感しにくいため納得感が薄い)
-
なんでこう書くべきなのか、よくわからない・・・
- (アーキテクト経験や、丁寧な設計に基づく実装経験を積んだエンジニアは割と希少なので、本当にその書き方で正しいのか分からない)
なので「優先度は低い」「工数かかる」「納得感が薄い」「正しいのか分からない」と、やりたくないと思う気持ちが膨らみやすいのがWHEREに対する指摘です。
必然的に「今はやらなくても良いのではないか」「いや、むしろこう書いたほうが良いのではないか」など議論が(PRのコメントも)膨らみやすい傾向が伺えました。このままでは時間が勿体無い!
全体で共有しよう!
そこでPrAha Inc.では「PR道場」なるものを週次開催するようになりました。
- テーマを絞る
- そのテーマに関する指摘を受けたら、全体に共有する
- 必要に応じてモブプログラミングを行い、手を動かしながら認識を揃える
弊社では前述の通り「WHERE」にテーマを絞り、WHEREに関する指摘を誰かが受けたら、毎週1~2時間しっかり時間をとって、全員の認識を合わせます。この時は全員が内容を知っている(はずだった)DDDに準拠して議論を行いました。チームの意見が割れる傾向にある場合、何か一つ寄る辺を用意しておくのは議論を素早く進めるために重要だと感じます。
特にWHEREに関するテーマは「手を動かしながら認識を揃える」のが重要です。概念だけ覚えても、いざ手を動かそうとすると「あれ、こういうケースってどう書けばいいんだ・・・」と迷い始めるもの。擬似コードでも構わないので、実際に一つユースケースを実装してみると、驚くほど多くの認識違いが浮き彫りになります。自分はDDDを学んだのではなく、学んだつもりになっていたのだなと、この時思い知りました。
僕らの場合、初回は2時間かけて会話しても認識が完璧に揃わず、サイゼリヤで夜ご飯を食べながら議論を続けるような状態でした。各々が過去にDDDに関する書籍などは読了して、それなりの基礎知識は持っていたものの、いざ実装レベルで認識を合わせようとするとこれほど乖離するものかと驚いた記憶があります。
どれだけ議論してアーキテクチャを整えても、全員がそれを実践できなければ意味は無い。それどころか議論に時間を食いつぶすだけで効果が薄い状態になり得る事を学びました。
明文化して、残す
特に新規参画者は新しいアーキテクチャに不慣れなので、こうした議論には置いて行かれがちです。新規参画者には議論を図解して後日発表してもらうなど、整理役をお願いする事でキャッチアップが図れるのではないでしょうか。
弊社でDDDに関する認識合わせを行なった際には、新人エンジニアがこんな図を書いてくれました。(こちらは初版のため、この後CQRSを導入したり、何度か修正を加えています)
こうした図と合わせて、以降PRに対して受けた指摘や議論した内容は全て専用のレポジトリに書き残し、VuePressで作った静的サイトとしてFirebaseでホスティングしています。
新規参画者には「とりあえず実装に悩んだらココを見てね」と伝えておけば、ある程度の認識は揃えられます。
ちなみに「ルールに違反するPRを出した人はコンビニにパシらせるぞ!」というルールを作ったところ、僕が真っ先にパシらされました。冬空の下でコーラを買いに走った悔しさ、忘れません。
効果
認識合わせに長い時間を費やしたものの、PRでの指摘が大幅に減り、体感的に1ヶ月程度で元は取れたと感じます。その分「WHAT」に注目してレビューを行えるようになり、仕様ミスなど、あってはならない失敗を補足する余裕が生まれました。チームが「HOWやWHAT」の指摘に終始するうちは別の解決策が必要かもしれませんが、「WHERE」に関する議論が多いようであれば、もしかすると参考になるかもしれません。
まとめ
- コードレビューが使い捨てになっていないか?
- 自分たちのチームで共有すべき集中テーマを見つけて、共有する事をオススメする
- 最低限の開発力を身につけたチームであれば、指摘点はWHEREに集中する気がする
- PrAha Inc.ではWHEREに関する指摘を週次で全体に共有する「PR道場」を開催した
- 体感で1ヶ月程度で元は取れている気がする
個人的にエンジニアとして一番楽しいのは、勉強熱心な仲間と切磋琢磨しながら新しい事を学び、活かせた瞬間です。こうした組織風土のあるチームであれば、PR道場のような取り組みは有効なのではないでしょうか。学ぶ心がある限り、PR道場は門戸を開き続ける事でしょう。(綺麗に〆た!)