良いコードレビューの文化のために
はじめに
コードレビューはしていますか？
コードレビューをする側になったときに「ま、いっか」で済ませていませんか？
品質の高いプロダクトにするために、 Pull Request が出てコードレビューする立場になったときに注意すべきことをまとめました。
〜〜すべきだ、〜〜するべきでない、といった口調になっていますが、個人の見解なのでご了承ください。
※この記事は、社内LTの内容について加筆修正したものです。
コードレビューにおいて何よりも大切なこと
以降で５ヶ条としてコードレビューの際に気をつけるべきことをあげていこうと思っているのですが、それらの何よりも一番大切なことを1つ上のレイヤーとして述べます。
それは、「コードレビューはコードのレビュー」だということです。
ときには有り得ないようなコードに出くわすこともあるでしょう、どうやったらそんなコードが生まれてくるのか純粋に疑問を持つこともあるでしょう、黒魔術のようなコードだってたくさんあります。
しかし、どのようなコードであっても、あなたがすべきことはコードに対してレビューすることです。
決してコードを書いた人に対してレビューすることではありません。ましてや人格否定すべき場所でもありません。
後輩だろうが先輩だろうが新卒だろうが中途だろうが社員だろうが業務委託だろうが、相手への尊敬の念を持って、コードに対して向き合いましょう。
コードレビュー5ヶ条
前述の何よりも大切なことを踏まえた上での5ヶ条です。
無慈悲に
この処理は無駄が多すぎるからこうした方が良い
コードレビューは無慈悲にするべきです。
「ま、このくらいいっか」と思いコメントしなければ、そのコードは半永久的にそのままです。
品質にレベルのようなものがあるとすれば、上限レベルは明らかにそこで止まってしまいます。
そのため、思ったことはどんどんコメントしましょう。
賛否両論あるかもしれませんが、個人的には些細なことでもたくさんコメントすれば良いと思っています。
大局に影響は与えないような箇所への些細なコメントから致命的なバグになりえる箇所への重要なコメントまで、遠慮する必要はありません。
ただ、全て同じようにコメントしていても、レビューを出した側には重要性が伝わらない可能性があるため、理由も含めてプレフィックスなどと一緒にコメントするルールにするとより良いかもしれません。
例1: [提案]このメソッド名がわかりにくいのでhogehogeなどにした方が良いかもしれません 例2: [要修正]このメソッド名と同じものが公式にあって被っているのでhogehogeなどにしましょう
甘えを捨て去り、思いのたけをコメントすることで、後々はそれが全員のためになるのです。
レビューを出す側のあなたへ
どれだけコメントされようが知ったこっちゃありません。へこたれる必要は1ミリもありません。
1つ1つを読み、対応すべきだと思ったコメントのみ対応すれば良いのです。
悩ましいコメントであれば、コメントででもチャットででも対面ででも議論してから決めても良いのです。
中には乱暴で間違ったコメントもあるかもしれません。
律儀に全てのコメントに対応していては、良くない修正を加えてしまうかもしれません。修正した方が良いコメントを見極めましょう。
中には今回の Pull Request の範疇に収まらないコメントもあるかもしれません。
確かに修正すべきだけど優先順位が高くないと思うなら、issue等に記録しておき後日対応にしましょう。
棚に上げる
(自分はできてないけど)コメントが少なくて追いづらいからもう少し増やしたら？
コードレビューする際には自分のことを棚に上げるべきです。
コメントする内容について、それを自分自身でできている必要はありません。
むしろ、「ああ、普段は俺はちゃんとできてないけど、ここはこうした方が良いなあ」と思いながらコメントする方が、自分自身の戒めにもなり成長するキッカケにもなります。
そのため、コードレビューでは自分のことは忘れて、それをすべきだと思うならどんどんコメントするべきです。
どうせ自分のことを忘れていられるのは今だけです。近いうちに立場は逆転して同じコメントをされるようになります。
こうしてチーム全員の底上げにつながるので、後々はそれが全員のためになるのです。
レビューを出す側のあなたへ
「お前こそできてないじゃないか！」と思うようなコメントもいっぱい来るでしょう。
しかし、そのコメントが正しくて修正すべきだと思うなら素直に修正しましょう。
そして、同じコメントをし返してやりましょう。
提案する
全然良い案はないけど、この冗長な処理はもっとスマートに書けそう
コードレビューでは具体案ではなく提案するだけでも構いません。
例えノーアイディアであっても何かこうした方がいいんじゃないかと思えばコメントすれば良いのです。
そうすれば、レビューを出した側は多少なりとも考えるキッカケとなり、もしかしたらベストな案が出てくるかもしれません。
もしくは、他のレビューしている人がそのコメントを見て良い解決策を提示してくれるかもしれません。
大切なのはキッカケです。
ただし、無責任で投げやりで適当なコメントはやめましょう。線引は難しいところですが。
提案してそれが結局は通らなくても問題ありません。
問題提起など提案することが大切です。
その提案内容が良いことか悪いことか、具体的な案は何なのか、それはコメントしたあとに議論できれば良いのです。
考えるキッカケを提供することでより良い品質になる可能性が高まり、後々はそれが全員のためになるのです。
レビューを出す側のあなたへ
無責任だなと思うかもしれません。面倒くさいなと思うかもしれません。
でも、あなたは素晴らしいキッカケを得ることができたのかもしれません。
それが無用な提案であれば突っぱねれば良いのです。
ハッと閃き良いコードに落とし込めれば御の字です。
別のレビュー者があなたの代わりに素晴らしい案を出してくれれば、それはもうタナボタ最高です。
いずれにしても、最初のコメントがなければ起きえないことです。
質問する
この処理は削除されてるけど、もう不要ってことですか？
コードレビューのコメントは指摘だけではありません、質問することもコメントのうちです。
何かわからないことがあればどんどん質問しましょう。
わからないということは必ずしもあなたの知識不足というわけではありません。
コードが読みにくい、コードにコメントが足りない、ドキュメントが不足している、そもそも仕様が良くない、あらゆる要因があって「わからない」が生まれています。
「聞くは一時の恥、聞かぬは一生の恥」と言いますが、一時の恥ですらないのでどんどん質問しましょう。
質問することで、あなたは新しい知識を得られるかもしれませんし、コードにわかりやすいコメントがつくかもしれませんし、ドキュメントが整備されるかもしれません。もしかしたら、的はずれな質問かもしれません。
いずれにしても質問したことで、それが記録となり、後から見た人（将来の自分や将来のレビューを出した人も含む）の理解を助けるのは間違いありません。
質問が起点となり、後々はそれが全員のためになるのです。
レビューを出す側のあなたへ
こんなこともわからないのかよと思うでしょう。
確かにまだ知識不足な後輩かもしれません。でも、もしかしたら、あなたのコードが読めたもんではないのかもしれません。
少なくとも1人にとってはわからない部分があったということを真摯に受け止め、教えてあげるなり補足するなり、前向きな対応をとりましょう。
褒める
メソッドの分割が適切で、可読性も高くなっていて、とても良いです！
コードレビューのコメントは指摘だけではありません、褒めることもコメントのうちです。
どんなに天邪鬼であっても人は褒められると嬉しく思うものです。
特に自分で頑張った部分を褒められるとやって良かったと思います。
5ヶ条のこれまでの項目をそのまま実践すると、コメントがまあひどいことになるかもしれません。
コードレビューはコードに対するものであって人に対するものではないということがわかっていても、全く気にならないということはありません。
10個のコメントうち1個でも褒めコメントがあればレビューを出す側の気持ちも多少は和らぐのではないでしょうか。
褒めるコメントというのはただの馴れ合いではありません。
褒められた箇所は何かしら良いポイントがあるわけで、その共有に繋がります。
そして、そのポイントに気をつけようと思うだけで、褒められるほど良いコードが蓄積されていくことになります。
そうして、褒めるだけで、後々は全員のためになるのです。
レビューを出す側のあなたへ
気恥ずかしいでしょう。でも、嬉しいでしょう。
自分の頑張りが目に見えてあらわれているわけです。
褒められるために良いコードを書こうというのは浅はかかもしれませんが、少しのモチベーションにはなるでしょう。
忘れてはいけないのは、あなたも褒める番が来たら褒めるということです。
まとめ
何よりも大切なことは、コードレビューはコードに対するレビューだと認識すること
コードレビュー5ヶ条
無慈悲に
- レビューに甘えは不要
- 思いのたけをコメントする
- それが全員のためになる
棚に上げる
- 自分ができている必要はない
- すべきだと思うならコメント
- それが全員のためになる
提案する
- ノーアイディアでも問題ない
- 良し悪しはコメントしてから
- それが全員のためになる
質問する
- 「聞くは一時の恥」にもならない
- コメントが記録になる
- それが全員のためになる
褒める
- 人は褒められると嬉しい
- より良いコードが蓄積される
- それが全員のためになる
