コードレビューって何を見れば良いの？

なにこれ

筆者が、コードレビュー3年やってみて思ったことをまとめてみる！
要は備忘録というやつ。

この記事のゴール

以下をまとめられたら、コーヒータイムにしたい。

• 現時点で良いかも？と思っているコードレビュー方針
• レビューで工夫しているところ

筆者どのくらいレビューしている？

初めてソースコードレビューしてから3年くらい。
大体1日２〜３PRレビューしているっぽい。

本題

レビューでどのようなところ見ているか

ざっくり4つ！

• きちんと仕様通り動くのかな
• パフォーマンスに影響ないかな
• 他の技術者が見て理解できるコードかな(可読性)
• もっと良い書き方がないかな

前提: レビュー前の擦り合わせ

レビューイにどのような観点でレビューするかあらかじめ擦り合わせておく。
ルールでなく、考え方を伝えるのが良いなと思っている。
(採用しているアーキテクチャ思考など)

コーディング規約は作っても守るのがしんどいので、可能な限り自動チェックに任せてしまう。

レビュー観点1：そのコードきちんと動くの？

最初見るのは、やっぱりここ！

• 条件分岐漏れがないか
• 既存機能に影響がないか
• 仕様漏れがないか

レビュー観点2: パフォーマンス影響

一度運用が始まると直しづらくなるのがDB、キャッシュ周り。

• トラフィックの多い機能のキャッシュやDB周りに重い実装がないか
• レコード量が多くなるであろうテーブル関連処理に重い実装がないか
• JOINが多く複雑なSQLがないか

適切にINDEX貼ったり、DB設計見直したり、キャッシュ化してDB負荷下げたりを提案することが多いかも

レビュー観点3: 可読性が確保されているか

開発者が初見で見てある程度わかるかも？というレベルまでは確保したい。

• 関数名とロジックが一致しているか
• 関数は一つの役割のみを持っているか
• ドメインが適切な粒度で分けられているか
• ドメイン同士が密結合になっていないか
• イレギュラーフローの場合、コメントなど説明が記載されているか

コードの理解しやすさと、変更しやすさをレビューで担保できれば開発効率が上がるかも？

レビュー観点4: もっと良い書き方の提案

お願いベースで、こんな書き方あるけどどう？と聞いてみる。
メンバーがどんな考えで書いたのかが聞けることがあるので、煙たがられない程度に提案してみる

レビュー観点のまとめ

最初は、観点ごとに1周、２周、３周、4周と周回してコードを見ていけば、レビュー漏れも少なくなるはず。
レビューイにも、喜んでもらえそう！

レビューで工夫しているところ

• 何のためにレビューしているのかの定義
• 機能によってレビュー粒度を変える
• 気持ちよくレビュー依頼を出してもらう

何のためにレビューしているのかの定義

レビューってやると何となく良いし、かっこいい！と最初は思っていたのだが、きちんと目的を決めないと、「俺が考えた最強にかっこいいソースコードの書き方」を押し付けてしまうだけになるので、言語化しておいた方が良い。

レビューよりも実装した機能を適切な品質で早く提供したいので、過剰品質になってしまってもよくない。

例えば、下記な感じ

• チームメンバー間のコード品質を、サービス運用に支障がないレベルで維持するため

機能によってレビュー粒度変える

例えば。。。

キャンペーン系など、期間限定で使用するコード: 仕様満たしてればOK(レビュー観点1のみ)

決済、ログインなど重要機能: 今後長く使うので、じっくり確認(レビュー観点全部)

気持ちよく依頼してもらう

あくまでソースコードのレビュー

命令口調ではなく、頼み事する感じでレビューコメントすると柔らかくて良い。
どのメンバーから依頼されても同じように対応する！

フォローアップを忘れずに

歴の浅い方やジョインしたてのメンバーの場合、どうしても指摘項目が増えてしまうと思う。
指摘事項が多い場合は、口頭でコミュニケーションを図ると改善する場合が多い！かも？

まとめ

普段、そんなに考えずにレビューしてしまっているが、言語化すると結構考えてるじゃん俺！となって自己肯定感が上がる↑↑！ LGTMだよ！

レビューは誰でもできる工程なので、メンバーを巻き込んでコード品質をあげていけたい所存！