📃

コードレビュー観点表を作った話

2024/08/07に公開

はじめに

今回は、コードレビュー観点表を作った話について少し書かせていただきます。

社内ではGitHubを用いてコードレビューを行っていて、バックエンドの開発においては、コーディングガイドラインも策定しています。

しかし開発において、ガイドラインに書かれている事項が全てではないため、コードレビューを行う際のポイントが自分の中で綺麗に整理しきれていませんでした。

また、ガイドラインの重要なポイントを十分に把握できず、効果的なコードレビューができていない現状がありました。これを改善するために、コードレビューの観点表を作成したことで、コードレビューの質が上がった話についてお話ししようと思います。

一貫性がないレビュー
毎回レビューを行う際に、自分の中のレビューポイントが明確に決まっていなかったため、的確にレビューができていないこと

レビューにかかる時間が長い
自分の中でのレビューポイントがまとまっていないため、レビューに時間がかかっていたこと

単純なレビュー漏れ
上記の二点も相まって、レビュー漏れが発生して開発の手戻りリスクが上がっていたこと

など。。。

いくつか問題が出てくる中で、自分なりにコードレビューの観点をまとめてみました。以下すべてではありませんが、観点表の一部を抜粋してお見せします。

区分	観点	優先度	コードレビュー基準	補足
社内標準	安全性,性能	MUST	テストを書く	詳細はテストコード運用書を確認
社内標準	安全性	MUST	パスワードなどのセンシティブな情報をログに吐かない
仕様		MUST	仕様に沿った開発ができているかどうか
言語共通	可読性,保守性	MUST	コードが複雑になっていないか	循環的複雑度など、linterでのエラーがある場合は修正
言語共通	可読性	MUST	マジックナンバーを書かない
言語共通	可読性	MUST	他の人が理解し難い命名をしない
言語共通	安全性	MUST	不正な値が入らないようにバリデーションを行う
言語共通	安全性	MUST	握りつぶしている変数やerrorをなくす	エラーハンドリング箇所などで検索してreturn していないところがないかなどを確認
言語共通	安全性	MUST	適切なエラーメッセージを返せているか
言語共通	保守性	MUST	実装は適切なレイヤーに実装されているか
言語共通	安全性	MUST	scope以外の範囲をPRに入れていないかどうか
言語共通	パフォーマンス	MUST	loop内でdbの処理を行っている場合は、別の手段を考える	呼び出し回数が上がりパフォーマンスが下がる
...	...	...	...	...

実際に自分で運用してみたところ、レビューが効率的に行えるようになり、レビューの速度も大幅に改善することができました。観点をそれぞれ分類することで、頭の中が整理整頓されてレビューしやすくなった感じがありました。

問題になっていた、一貫性がないレビューを行なっていたことで、レビュー漏れによる手戻りリスクが上がっていたことも、要点を整理できていない場合と、観点表を見る場合とで比較すれば、下げれるようになったのではないかと思います。

また観点表に優先度を新しく加えたところ、この部分はMUSTで対応、こっちはWANTだよねっていうのが明確になり、さらにレビューしやすくなりました。「時間がなく急ぎでレビューしてほしい」などの依頼があった場合は、MUSTをチェックするなどしてさらに効率化が図れると考えています。

課題として残ったのが、コードの質をあげることは、すなわちレビュー観点が増えていくことにつながるのではないか？と思っていて、レビュー項目が増えれば増えるほどレビューにかかる時間も増えて、質はあがる一方生産性が下がることにつながるように思っていました。

linterなど一般的に使われているツールはあるかと思いますが、上記のような社内標準観点や言語共通観点を網羅的にチェックしてくれるわけではないので、今後レビューを自動化するなどのフローを考えてないといけないなと思っています。

コードレビューがしやすくなり、改善したい箇所が改善できて、作成して良かったと感じています。みなさんもぜひ自分でコードレビュー観点表を作ってみてはいかがでしょうか！！

CastingONE では、バックエンドを推進してくれるメンバーを大募集しています。カジュアル面談もやってますので、お気軽にご連絡ください！