コードレビューを研く June 3, 2017
valeblog
0 Comment

Vaidehi JoshiさんのCrafting Better Code Reviews記事の翻訳です。

人間と技術の関係はいつの時代でも単純や楽には決して言えない。特に技術を作る人にくると、それが明らかになる。私自身も、職業でコードを書く人として、もっともコードレビューの際に痛感する。

開発の人は作業をアートに近い目線でみて（そういうところは美術作家などのクリエーターと同様）、自分のコードへの愛着が非常に強くなりがちだ。開発はエゴを捨てるべきと、自分のコードだけじゃなくて見る機会があるマージ待ちコードはすべてしっかり見るべきだと言われる。書いたコードをみてもらうのも、同僚のコードをみるのも、スバラシイコト™でやるべきことだって。推奨されるそんな習慣を身につけてちゃんとやっている人も少なくはない。

しかしその方法論を最後ちゃんと振り返ってみたのはいつか？今のコードレビューのやり方で効果的だと、確信持っていえるか？元々導入した目的をちゃんと果たせているか？そしてもしそうでなかったら、どうすれば改善できるか？

© geek & poke

コードレビューやる意味ある？

コードレビューの実利を考える前に、その始まりを知っておいた方がわかりやすい。コードレビューの優践（ベストプラクティス）に関する研究はけっこうあるが、個人的にSteve McConnell氏のCode Complete（1993年初版）から始めた方がいいと思う。

コードレビューの解析、そして目的に関しても観察する。

ソフト開発のマネジメントの一環は問題を低値、つまり資源投入がまだ少なく修正費用が低い段階で見つけることである。そのために開発者が質関（クオリティゲート）を設定する：定期的なテストやレビューによって、品質が次の段階に進むに充分かを確認する。

McConnell氏がコードレビューの導入へのもっとも強力な論点は、それを”構築の共同所有”に結びつけることである。つまりすべてのコードは貢献者のチームが所有し、その全員が平等に閲覧・変更ができる。

コードレビューは元々、共同所有の理念を開発に当てはめようとして生まれた。品質をコントロールできることによって我々各自開発プロセスの利害関係者になる。

開発チームが日常的な業務に当てはめられるようなコードレビューの種類も何個か紹介する。Code Completeを読んでおくのをぜひおすすめしますが、ここではまとめだけにする。コードレビューの方式は下記の3通りだ。

1. 検査

検査は1時間前後かかる長めのレビューで、コード作者、検査員とモデレーターの3人で成り立つ。

効率的に進めると、一般的には検査で欠陥（バグやミス）が60%ほど発見される。McConnell氏の調査によると、そこまで正式ではない他の方法と比べれば1000行あたりの欠陥を20-30%減らす効果がある。

2. 読み合わせ

読み合わせ（ウォークスルー）は30-60分の業務会議で、普段は先輩が後輩に理論などを説明する機会でありながら、若手が古い方法論を批判できる場でもある。

場合によってはウォークスルーも効果的ではあるが、検査などより正式なレビューには及ばない。ウォークスルーでは一般的に欠陥の20-40%が発見される。

3. 手短なレビュー

手短なレビューは、その言葉とおりより速い。それでも小さな変更や、もっともミスしやすい1行変更に対する徹底したレビューだ。

McConnell氏は手短なレビューに関して下記の発見をした。

某組織が一行変更のレビューを導入して、エラー発生率が55%から2%に激減したことがわかった。80年代の某通信企業では導入によって正確さが86%から99.6%に上がった。

そのデータ（McConnell氏が扱うデータ）からは、どの開発チームにも上記3種類のレビューの何らかの組み合わせを導入すべきと言える。

しかしCode Completeは1993年に書かれ当時の研究に基づいている。以降も業界そのものそして我々の査読の方法論も変わりつづけてきた。今日のコードレビューは本当に効果的？理論をどのように行動に移しているか？

答えを見つける決心がついた（ただ確信は弱い）開発者として、インターネットに聞いてた。訂正：ツイッターに聞いてた。

開発者はコードレビューをどう思っているか

調査の結果分析に移る前に言っておくが、私は情報科学者ではない。（そうだったらいいけどね、分析もうまくできたし、Rとかでグラフまで作れた。）つまりデータが色々と限られている。ひとまずは自発的に答えてもらったツイッター上の調査で、そしてブランチ・プルリクを運用しているチームを想定している。

ではさっそく：開発者はコードレビューをどう思っているか？

量的なデータ

まずは量的なデータをみてみよう。

質問への回答は、どの開発者に聞くかによって大きくかわる。現時点では計500強の回答をもらっている。

回答者の中ではJava、RubyとJavascriptを使っている割合が一番多かった。使用言語などと回答内容はどういう関係にあるかみてみよう。

まずは「コードレビューはチームにとって有益」と思っているか、1~10で点数を付けてもらった。1は「一切そう思わない」で10が「完全にそう思う。」

コードレビューをもっとも有益に感じているのは平均9.5点でSwift使用者で、2位はそれに近い9.2点のRubyだった。

回答者の大多数（約70%）がすべてのプルリクがマージされる前にチームのだれかのレビューが入ると答えた。約50人（回答者の10%ほど）は頼まれた時のみレビューが入るという。

使用の言語やフレームワークに関わらず似たような割合がみえる。全プルリクがレビューされるか頼まれたもののみかに関しては目立った差がない。つまりコードレビューの姿勢には使用言語・フレームワークではなく、チーム自体が影響すると思われる。

そしてレビューが必須なチームでも、プルリクのマージには一人のレビューだけで充分というのが多数だった。

質的なデータ

では量的には測れないものはどう？調査では多項選択質問の外、文章で答えるものもあった。その回答がもっとも啓蒙的かつ効果的だった。

全般的にみられる傾向は何個かあった。

最終的にコードレビューの成功を左右するのは2つ：レビューにかかった努力と、レビュー自体の内容。

コードレビューが悪かった（レビューする人、される人問わず）というのは、努力もしくは内容が適切ではなかったということだ。一方時間がかかり、実質的な内容を持った徹底したレビューは関わった人にいい印象を残したという。

ここでいう努力と内容は一体何だろうか。

努力

レビューへの努力はこの2つの質問で測られるようだ：誰がレビューをするか？そして、レビューにどれだけの時間をかけているか？

回答者のほとんどがコードレビューに関わったが、多くがそのレビューをする人や、レビューにかかった（待たされた）時間に対して不満があった。

それに関する匿名返答を何件かまとめた。

すべてのプルリクにいいねだけ押してコメントも残さない人がいる。マージには最低限2人の承認が必要というルールをくぐろうとしている。何でわかるかというと、１分でプルリク5-6件も承認しているからだ。

2人目や3人目のレビューする人はもう安易に承認してしまう傾向がある。

誰がプルリク出したかによって同じコードが違う目線でレビューされることがあった。

チームの皆が同じ基準でレビューされるべき。上の人のプルリクがまともにレビューされないことがよくある。どうせミスしないと思われるだろうが、実際にミスもするし意見も聞きたいことがある。逆に若手はケチつけられる…自信に影響するし、特に立場が弱い若手は。

コミットが大きすぎて、レビューに時間がかかりすぎる。だれもローカルに落としてチェックしない。

大きなプルリクにけっこう時間がかかったりする。今後の作業に影響が大きいからそれは問題だ。

コードレビューにかかった努力に関して最終的に言えることは、

  1. 意味がない、形式だけのレビューはだれもうれしくない。
  2. 詳しくない箇所や背景が分からないプルリクのレビューは苦痛。
  3. 人間はミスをする。皆人間だ。皆平等にレビューし、されるべき。

内容

レビューの内容はつまり、レビューにあたって何を言って、何をして、他人をどういう気持ちにさせているかということだ。

レビューの内容に関する回答の多くが、レビューで何を言われたかそしてその言い方に関するものだった。

匿名の回答何件か。

プルリクへの反応そのままで受け止める。その文字列をシンボルに変えればあなたが満足するな、そうするからとっととやろう。特に理由もない判断に関して議論するつもりはない。IDEを使うと似た感じで、すぐ「赤い×発見。赤い×修正」の考え方に落ちてしまう。なんでぶうぶう言われるかはどうでもいい、黙らせたいだけ。

レビューで全体図や構築論をかたるな。直接会話して。混乱やイライラの原因になる。

修正を要求されるのをうざく感じる。特に裏付けも明記せず、間違っている余地まで残したら。特にコメントでコードを書き直されて、代わりにそれを使えと言われたら。

スレが長くなり始めたら、それは直接会話した方がいいしるしだ。結果だけスレに書いとけばいい。

様式の趣味と、本質的な指摘の違いを考えて欲しい。若手はそれを見分けるのが難しかったりする。そして2人の先輩が矛盾すること言うとイライラする（例えばルートの定義）。

レビューの内容に関してはつまりまとめると：

  1. 文法のケチ付けは不満を生む。形式は本質ではない。（おもしろいことに、回答者の5%がレビューへの不満を表すために「ケチ付け」という言葉を使った。）
  2. レビューの言葉使いが実際に影響力がある。ひどい言い方をしたレビューは自信を崩壊させる可能性がある。

改善するには

このデータは業界のレビュー文化の全体図として完全でも完璧でも正確でもないかもしれないが、確信して言えるものはある：各々のチームやコミュニティでのコードレビューのやり方を振り返って見直して損はない。

レビューが持つすさまじい影響力を示す回答もあった：

よくないコードレビューで会社を辞めそうになったことがある。そして素晴らしいレビューによって将来の課題に挑戦する力が身についたように感じる。

この調査でもMcConnell氏のでも何らかの正式なコードレビューが統計でも現れるように非常に効果的だ。ただし導入して放置するではいけない。機械的になってしまったレビューは逆効果をもたらす恐れがある。

内省、振り返りと見直しがコードレビューの文化の進化を支える。

すなわちコードレビューの今のやり方で効果的か、各自でもチームとしても常時振り返る必要がある。

手っ取り早い改善方法

コードレビューを苦痛から楽しいことにするための手っ取り早い方法はある。例えば：

  • 可能な限りコード分析や掃除ツールを使用。文法や書き方に関する指摘がなくなる。
  • ギットハブのテンプレを使う。チェックリストなどがあると、作成者も審査員も何をすればいいかわかりやすくなる。
  • 背景に詳しくない人のためにスクショや細かい説明を追加する。
  • コミットやプルリクをこまめにすることでレビューはより楽に、早くなる。
  • プルリクにレビューする人をできれば複数人指定する。どのレベルの人にもレビューするチャンスをあたえる。

より難しい（そしてもっとも重要な）ところ

上記のような、簡単に解決できる問題を片付けてからもっと大きな変革も視野に入れられる。コードレビューの文化を改善するために影響力は大きいが、もちろんその分楽にできることではない。

  • チーム内で共感できるようにする。これは主に上級の人の責任になる。新人や若手の視点を考慮するよう努力する。
  • 立場の弱い人も居場所、発言権がある環境を作る。コメントの言葉使いに注意して、レビューがよくない方向に傾き始めるのをみて、公に他人への疑問を発言するより直接会話を求めるなど。
  • 会話をする。チーム会議をするなり、スラックでスレを立てるなり、匿名調査をするなり、文化にあった方法を取る。皆がレビューのプロセスに関して意見を気軽言えるような空気を作る。

もっとも重要なのを最後に残した。なぜならここまで読んでくれたら本気で変革を求めている。それはまさにスバラシイコト™。だが結局実践に移す第一歩としては会話が必要。

この回答が一番うまくまとめてくれている：

理論上コードレビューが大好き。だが実践に移すと、そのチームのやり方で効果が決まる。

資料

この調査のために作ったサイトでもっと詳しく匿名化した回答をみることができる。

謝礼

真っ先に回答してくれた何百人の開発者に礼を言いたい。

回答の解析を手伝って、グラフを作ってくれたKasra Rahjerdiに熱く感謝する。

そしてJeff Atwoodの査読に関する研究資料、Karl WiegersのHumanizing Peer Reviews、そしてCode CompleteにつながったSteve McConnellの徹底した研究に感謝しないといけない。彼らを応援して書籍を買ってくれたらと思う。

翻訳者からの一言

最近本業とも言えるプログラミングの職に着いて、dev.toをフォローし始めた。毎日考えさせられるような記事ばかり。翻訳した記事ももともとそこでみた。

なんで訳そうと思ったかというと、ちょうどこの前コードレビューでイラっとしたからだ。まさに調査の回答者もよくいう、スペルチェックされるためにコードレビュー頼んだ訳じゃない。コードの本質、ロジックなどに不安があるからしっかりみて欲しかった。

また、Code Completeの和訳が（まだ）手元にあるわけではないので、自分なりの最善の訳ではあるが、書籍の文脈と必ずしも一致しないかもしれない。

実際にこの内容を仕事にいかせるかは分からないが、やってみる価値は確実にある。