POSTDの【翻訳】ピアコードレビューの実践的レッスンという記事に実務に取り入れられそうなポイントがたくさん盛り込まれていました。
備忘録として引用とコメントを書き残しておきます。
なぜコードレビューをするのか？
最初に、なぜコードレビューをするのかを思い出しましょう。プロのソフトウェア開発者にとって最も重要な目標の1つは、ソフトウェアの品質を常に向上させ続けることです。たとえあなたのチームメンバが才能あふれるプログラマばかりだとしても、チームとして働く限り、有能なフリーランサーであることを切り離して考える必要があります。コードレビューはソフトウェアの品質を向上させる最も重要な方法の1つです。特に、以下の点において重要だと言えます。
- 不具合やより良い方法を見つけ出すための客観的な視点が得られる。
- 担当者以外に少なくとももう1人、コードの内容を理解する人を確保できる。
- 経験豊かな開発者のコードを見ることは、新人の訓練に役立つ。
- コードのレビュアもレビュイも、他のメンバの良いアイデアやプラクティスに触れることで知識の共有ができる。
- 同僚のレビューを受けると分かっているので、開発者は自分の仕事に対してより完全なものを目指すようになる。
箇条書きになっている重要な点、全てに同意です。
コードレビューはコストが掛かるけど、エンジニアとデザイナーが１０人近く関わるプロジェクトにおいてはメリットの方が多いです。今関わっているプロダクトは、開発者が数人の時代は重要な機能以外はコードレビューせずに、どんどんプロダクト開発を進めていました。しかし、１０人規模になってそういうフェーズは終わったと感じています。
致命的な問題が起きる確率を下げるためにレビューは有用な手段です。
徹底的なレビューの実施
しかし、適切な時間配分とともに相応の注意を払ってレビューを行わなければ、上記の目標は達成できません。パッチに一通り目を通して、インデントが正しいか、全ての変数がローワーキャメル記法になっているかということを確認しただけでは、徹底的なコードレビューとは言えないのです。よく使われるプラクティスであるペアプログラミングの手法を検討してみることは有益でしょう。この手法では、基本的に開発と同じ時間をかけてコードレビューをするので、所要時間は全体の開発時間の100％増となります。このペアプログラミングに比べれば、どれだけ時間をかけてもエンジニア1人がレビューに費やす時間はずっと少ないのですから、コードレビューには十分な時間をかけることができるはずです。
私の考えでは、本来の開発に費やす時間のおよそ25％をコードレビューにかけるのが妥当だと思います。例えば開発に2日費やすとしたら、レビューには約4時間かけるべきだということです。
コードをレビューする際には、コードの不具合やその他の問題を探すだけではなく以下についても確認すべきです。
- 必要なテストケースが全てそろっている。
- 適切な設計書が存在している。
“本来の開発に費やす時間のおよそ25％をコードレビューにかけるのが妥当”という考え方は、開発工数を算出するときの目安にしています。
コードレビューの過重な負担を防ぐ
チームがコードレビューを義務化して行っている場合は、コードレビューのバックログが管理できないレベルにまで膨れあがってしまう危険性があります。2週間、全くレビューを行わなかったとしても、レビューの遅れを取り戻すための数日間を確保するのは、難しいことではありません。しかしこの状態では、いざコードレビューを行った時に、開発作業が大きな予期せぬ問題に巻き込まれてしまうことになるでしょう。また、きちんとしたコードレビューを行うためには、精神的に集中した状態を保たなければなければならないので、レビューの質を上げるのも大変です。何日も続けてこの状態を保つことは困難でしょう。
そのため、開発者は毎日、レビューのバックログを消化するように努力すべきです。朝一番にレビューを行うことは、それに対処するための1つの方法です。未消化のレビューをその日の開発を始める前に行うことで、レビューしなければならないと思い続けながら開発する状況から逃れることができます。昼休みの前後や、仕事の終わりにレビューを行いたいと思う人もいるでしょう。いつやるにせよ、レビューを開発の邪魔だと考えるのではなく自分の毎日の仕事として捉えることで、以下に挙げるようなことを回避できます。
- レビューのバックログを消化する時間がない。
- レビューが終わっていないためにリリースが遅れる。
- コードの内容が既に変わっているのに、古いコードについて指摘してしまう。
- 大急ぎでレビューをしたために、不十分なレビューになってしまう。
先週から、”朝一番にレビューする”という習慣を取り入れました。
レビューしないといけない Pull Request が２０件近くあるという不健全な状態なので、毎日コツコツとレビューとマージをしていくことがプロダクトを健康に保つために必要だと思ってます。
言うまでもないですが、プルリクが２０件とか多すぎるので片手で数えられるぐらいにしたいです。
レビューしやすいコードを書く
もしアーキテクチャに複雑なところがあるならば、事前にレビュアに会ってそれについて議論しておくことは道理にかなっているでしょう。これによりレビュアは、コードが何のために書かれていて、機能がどのように実現されるのかが分かるので、コードをより簡単に理解することができます。また、コードを書く側にとっても、レビュアから別のより良いアプローチを提案されたことにより、コードの大部分を書き直さなければならないというような状況を回避することができます。
レビューの段階で大きな手戻りが発生しないように、事前にレビュアーやチームメンバーと仕様について議論しておくことは大切なことです。
大規模なコードのリファクタリング
最善の解決策はコードのリファクタリングを徐々に行うことです。
これに加えて、大規模なリファクタリングをするときは必ずチームメンバーにリファクタリングに取り掛かるということを情報共有するべきです。
論争の解決
あなたのチームは間違いなく知的なプロで構成されていて、ほとんどの場合、特定のコーディング問題について意見が異なっても合意できるはずです。レビュアが別のアプローチを好む場合に備えて、開発者として広い心を持って妥協する準備をしておきましょう。自分のコードに対して自分のものであるかのような態度を取らないでください。また、レビューのコメントを個人攻撃と受け取らないでください。あなたに対して、いくつか重複したコードを他の関数を再利用してリファクタリングを行うべきだと感じる人がいるからといって、あなたが魅力的でもすばらしい人物でもないと否定されているわけではありません。
レビュアとして気を利かせてください。変更を提案する前に、それが本当に優れた提案なのか、それとも単なる好みの問題なのかをよく考えましょう。議論すべき箇所を選んで、元のコードで明らかに改善が必要な箇所に論点を絞れば、もっとうまくいくでしょう。「ペットのハムスターの方がこれより効率的なソートアルゴリズムを書くことができる」と言うのではなく、「～を検討する価値があるかもしれません」や「～を薦める人がいます」という風に言ってください。
妥協点を見つけることができない場合は、両者が尊敬する他の開発者に見てもらって意見を聞くといいでしょう。
自分の仕事にプライドは持ちつつも、謙虚な姿勢は常に持っておきたいですね。
個人的にレビュアとして気をつけないといけないことは「単なる好みの問題」を押し付けていないか？ということです。
コードレビューは奥深いので実務経験を通して、知見が溜まったらまとめて記事にしたいです。
