Hatena Developer Blog

はてな開発者ブログ

コードレビューを会話しながら行う取り組み

こんにちは。アプリケーションエンジニアの id:itchynyです。

今回は、コードレビューを会話しながら行う取り組みについて紹介します。

コードレビューは大事なコミュニケーションの場です。 コードレビューの効用としては、単純なミスがあるコードをリリースしない・プロダクトのコードの品質をよりよくしていく、あるいはその方策を模索するといったことが挙げられます。

こういったことは当然のことですが、なによりもまず、レビューというのは一緒にプロダクトを作っている仲間とのコミュニケーションの場だと思います。 多くの人は、プロダクトのコードをよくしていきたい、読みやすいコードを書きたい、分かりやすいコードで目的の機能を作りたいといった共通の思いを持っていることでしょう。 コードを書いた人の思いを汲み取りながら、共感したり、譲歩したりしながら、よりよい方法を提示していきます。

それでも時には、どういうコードがきれいなのか、このコードは分かりやすいのか分かりにくいのかといった価値観の違いによって、衝突することがあります。 こっちのほうがより良い方法だ、なぜなら… そうかもしれないが、それはわかりにくすぎる… いや、有名なプロダクトでもこう書いてある… みたいなやり取りを何往復もやり取りしていると、疲弊してしまいます。 何往復も、ではなくても、一つのプルリクエストに10も20もここを直しなさいと言われると凹む人もいるでしょうし、指摘を書いているうちに心配になって文言を柔らかくしたりすることもあるかもしれません。

レビューしているコードが思っていたのとあまりに違う、 やりたいことに対して複雑になりすぎてそう、 フレームワークのマイグレーションなどで差分が大きい、 新しい機能を作りはじめたので他の箇所のコードと雰囲気が異なる。

こういう場合には、レビュワーと実装者が実装の意図などを会話しながらコードを読んでいくレビューを行うことをおすすめします。このスタイルを、ペアコーディングのレビュー版ということで「ペアレビュー (pair review) *1」と呼んでいます。

ペアレビューの進め方と大事なこと

ペアレビューでは、一つのスクリーンに (あるいはリモートだと画面共有して) コードの差分を表示し、書いた人とレビューする人が一緒に見ます。 レビューする人は、まずコードを書いた人から意図を引き出します。 「ここはこうなっているわけだけど… なぜかというと…?」「あーそうかなるほど、ここはこっちのコードを参考にしたんですね… ふむふむ…」というふうに、相槌を打ちます。 その後に、「確かにそうなんですけど、こうした方がよりよくないですか?」「このやり方でコード書くの大変じゃなかったですか?」といった具合に会話を続けます。 会話を進めると、相手の温度感が分かってくるわけです。 「いや、自分はこの書き方がよいと思ってます」という人もいれば、「そうです… ちょっと大変でした。何かいい方法ありますかね」という人もいるでしょう。 「そっちはこういう理由でこうなっているんですけど、今回こうしてしまうと…」「実はもうちょっといいやり方があって…」「そうなんですよね、私もこの書き方をしたことあるんですけど、こういうことで後で困ってしまうので…」といった具合ですね。 もちろん、レビューをする人の意見が正しいとは限りません。 「そうか、言われてみれば、この書き方でよさそうですね」 会話の中で、うまいところに折り合いをつけていきます。 「ああ、この書き方はめっちゃいいですね」 良いコードを褒めるのもレビューです。

会話をしながら徐々に方針が決まっていきます。共感ポイントをレビューコメントに順番に書き留めていきます。 もちろんそのプルリクエストは他の人も見ていますし、差分が長いとペアレビューを終えてからどう直すかを忘れてしまうからです。

ペアレビューの大事なことは、まず相手の意図を引き出すということです。 実装者の立場で、ある関数を使っているところを消したけど、その関数自体を後々のために置いておきたいというぼんやりとした気持ちがあったとします。 コードレビューで「この関数は使ってなさそうなので消しましょう」と書かれた時に、理由がぼんやりしているためにしぶしぶ消すか、あやふやな理由で反論するみたいになってしまいます。 こういったケースでも、レビューをする人は「この関数ってもう使っていないんでしたっけ?」に対する温度感を引き出すわけです。 消そうと思っていたけど忘れていた場合は「ああ、そうだった、消します」と返ってきますし、何らかの消したくない気持ちがある時は「残しておきたいんですよね」と返ってくるでしょう。 この温度感によって、どうするのがよいかを深掘りするわけです。

コードレビューは古いとか、毎回ペアレビューをしましょうとか言いたいわけではありません。 コードレビューを基礎に置きつつ、さらにコミュニケーションを活発にする一つの手段としてペアレビューは有効だと考えています (コードレビュー 9 : ペアレビュー 1 くらいの気持ちです)。 社内でも、大きな差分がある時にペアレビューをやってみると、実装の意図がいつもよりもよく分かってやってよかったという声がありました。 おもしろい例としては、デザイナーさん同士が (実装意図をいちいち書き込むことの少ない) CSSをペアレビューしたという例もありました。

いつもと違ったレビュースタイルとして、ペアレビューをしてみてはいかがでしょうか。

はてなのエンジニアとペアレビューしたいインターン生を応募しています。

developer.hatenastaff.com

*1:ピアレビュー (peer review) とは異なる