社内勉強会で「仕事でのコードレビューについて思っていること」というテーマで話をしました

10/23に社内勉強会で「仕事でのコードレビューについて思っていること」というテーマで話をしました。

スライドは前回同様に公開できる形で作ってSpeakerDeckで公開しました。

スライドには書かずに口頭で補足したことやQ&Aで回答したことなどを書いておこうかなと思います。

スライドの補足

コードレビューをしなくていい場合について

自分用のコードについて以外はネタなのであまり触れないでください。
チームで仕事する以上は基本的にコードレビューすべきと考えています。

レビューアーの選定について

僕のチーム（7名）では全員がレビューアーで承認は2名以上必須としています。
2名の内訳の制限はしていませんが、内容によっては詳しい特定のメンバーの承認を待つようなことはあります。
全員がレビューアーとしているのは、「特定の人へ負荷が偏らないようにするため」と「全員が多くのコードに触れる機会を作る（コードリーディングの時間を作る）ため」という2点が大きな理由です。

レビューイー・レビューアーの心得について

発表の都合でキリよく4つずつにしています。細かい話をすればもっとありますが今回は厳選しました。
大事なのはお互いに相手を尊重し相手に理解してもらうことに手を抜かないことだと思います。

コードレビューの認識合わせについて

チーム内でコードレビューの目的や観点や心得の認識合わせをしています。
また認識合わせした内容をまとめてチーム用のWikiに公開しています。

コードレビューでフォーマットについて議論することの是非について

基本的にはコードレビューでフォーマットについての議論は控えめにすべきかなと思っています。絶対にするなとまでは思わないです。あくまで控えめに。
コードレビューで延々とフォーマットについてコメントをするのもされるのも辛いのでフォーマッターなど機械的にできるものにお任せしたいですよね。
しかし、新人や経験の浅いメンバーに対しては個別にコメントしてフォローしていきます。
これはフォーマットが崩れいていることなどを気にする人になって欲しいという思いからです。単にルールとして決めているだけなのか、何か思想があってそうしているかなどのフォローもします。しているつもりです。

コードレビューが全てを解決するわけではない

コードレビューはすべてを解決する銀の弾丸ではありません。バグが0になったりはしません。バグは色々な工程で少しずつ解消するものです。

コードレビュー以前・以外にやるべききことはいろいろあります。コードレビューで議論が長引くようなときはモブプロ・ペアプロしたり、Zoomなどで直接会話したりします。また、仕様についての理解が浅いなどあれば仕様の読み合わせをしたり、必要な知識を補完するための勉強会をしたりしてフォローすることも大切だと思っています。

コードレビュー以外でのコミュニケーションもとても大事です。

Q&A

Q1. コードフォーマッターを実行していないという指摘をしたくなりませんか？

僕のチームではProntoを使ってGitLabのMerge Request上にRuboCopの指摘がコメントされるようにしています。CIに組み込みエラーにするのもありですが指摘数を0付近に保っていないと厳しいです。

Q2. コードレビュー時にプログラムの実行をして動作の確認まですべきですか？

基本的には動作確認はレビューイーの仕事だと思っています。
自分がコードを理解するために必要なら実行することもありますが基本は実行しません。動作するかはテストコードで担保されていると考えています。
もちろんGitLabのMerge Requestをトリガーにテストが実行されるように設定しています。
画面の改修などはスクリーンショットを貼ってもらうように依頼したりすることもあります。

Q3. 複数人でコードレビューするときは役割分担などをしていますか？

僕のチームでは特に役割分担はしていません。込み入った対応などは詳しい人に見てもらうことはあります。
また、内容によっては別チームにコードレビュー依頼をすることはあったりします。

Q4. コードレビュー時間はどれくらい使っていますか？

僕は残念ながらややレビューおじさんになっているような節があるのでコードレビュー時間は長いと思います。
平均すると毎日3～4時間程度でしょうか。
厳密に計測していないのでわかりませんが、コードに関わっている時間の8割程度はコードレビュー時間です。

Q5. コードレビュー時のコメントはすべてPull Requestに載せますか？Slackなどで会話しますか？

ややこしい内容や議論が長引きそうな場合にSlackやZoomや口頭で会話することはあります。そこで会話した内容はできるだけPull Requestに転載します。
特に「話題には出たが採用しなかったこと」や、「決定の経緯」などをコメントに残します。Pull Request上にそのようなコメントが集約されていると、議論に参加できなかった人や後で見返したときなどに有益だからです。

まとめ

コードレビューは手間はかかりますが良いコードに育てていくためにとても大切なことだと思います。
コードレビューでヘイトが溜まってしまうのは誰も幸せになれません。言葉遣いに気を付けたり必要な情報を提示するなど相手の負担を下げるための行為に手を抜かないことが大切です。

僕が日々の仕事のコードレビューを通じて思っていることのまとめです。
誰かの参考なれば幸いです。

楽しいコードレビューライフを！