コードレビューで意識していること
こんにちは。今回は私がコードレビュー時に意識していることについてまとめたいと思います。
1.好みを押し付けない
コードレビューでたまにあるのが、その人の「こだわり」による指摘で永遠に修正が終わらないことです。
私が「こだわり」になっていないか否か判断ポイントとしては、下記の感じです。
- ８割の人がこの意見に賛同するか？
- その人が今回の注意で今後気をつけることが出来るのか？
- 曖昧な判断基準になっていないか？
では、よくある例を見ていきたいと思います。
命名
タイポや英語的に分かりづらい、は勿論指摘OKだと思います。
ただし「こっちの方が命名良く無いですか？」みたいなのだと、レビュー依頼者は今後レビュー者の好みを予測して命名する必要があります。(そうしないとまた永遠と指摘が入ってしまう)
察してコーディングは超大変なので、それが「こだわり」になっていないかを意識して指摘する必要があると思います。
処理方法
メソッドなど、処理の方法は様々なアプローチがあると思います。勿論「この方がメモリ効率が良いと思います！」とか「今後こういう仕様が増える予定なので、こうしておいて欲しい」など明確な理由がある場合は指摘OKだと思います。
しかし、自分が想像していた方法と別の方法が書かれていた際に「こっちのほうが良く無いですか？」という指摘はしない方が良いと考えています。
2.挙動が問題ないか
これはサービスとして重要な点ですね。この確認をする際は
- 仕様を把握
- テストシナリオを頭の中で作る
- あらゆるパターンを確認
という手順で確認しています。ここでコーディングした人が見落としているパターンを見つけ出すのが至高です。また、これについては自分がコーディングしてレビュー依頼を出す前にも行なっています。(意外と挙動確認しないまま、レビュー依頼出す人も多いですよね…)
3.仕様は守られているか
これは挙動以外の部分の仕様が守られているか、という視点になります。
- デザイン
- 分析タグなど裏側の処理
などなどです。
ここで一番難しいのが「何に従うか」をはっきりさせてレビューすることです。
例えば
- 仕様について資料Aと資料Bの内容が異なっていた場合は、どちらに従うのか？
- デザインはこうなっているけれど、気持ち悪いから勝手に直してコーディングしました
などのことが発生した際、どうするのか。
こう言った場合は、必ず一番信用出来る資料を採用する or 各責任者に確認する という方法を取る必要があると思います。デザインであればデザイナーに従うし、仕様であればディレクターに従うし…
結構自分で判断し、改変してコーディングしている場合もあるので、他の資料と照らし合わせて確認をしています。
4.コーディングルールに則っているか
これは最初に書いた「こだわり」とはっきり分ける必要があります。
まずコーディングルールは１つに絞って、最初に提示しておく必要があります。「こっちのルールではこう書いてあります」とか言い始めると、キリがないし正解がなくなってしまうためです。
lintをガチガチに設定しておくのも手かもしれません。
5.理由を書く
最後に私がレビューで意識していることは、必ずコメントにその指摘をした「理由」を書くことです。
何故その指摘をしたのかを書いておくと
- 次回同じ間違いをしにくくなる
- 別のコーディングをした際にも活用できる
と思っているからです。
逆に理由が書けないような指摘はしないでおきます。直す理由が無いので…
まとめ
レビュー全体で意識していることは「判断基準を曖昧にしない」という点に尽きると思います。
毎度同じ判断基準でレビューをすること、全員が納得するようなレビューをすることを意識しています。
判断基準を曖昧にすると自分がコーディングしている時もどこに従うべきか、悩んでしまって自分の首を絞めることになると思っています。
ただ、PJによって勿論大切にしている部分は異なると思うので、郷に行っては郷に従えが一番大事ですね。
お読みいただきありがとうございました。
Discussion