はじめに
私がリーダーを務めるチームでは、未経験で入社した社員も3ヶ月もすればコードレビューを任せることになります。
今までは「過去のマジリク見てやり方学んでください」というスタイルでしたが、新人も増えてきてこのままではレビューの質に差が出てしまって良くないですね。
そこで今回、私がどんなことを考えてレビューしているかをまとめました。
対象者
この記事は下記のような人を対象にしています。
- 駆け出しエンジニア
- プログラミング初学者
- 上司から「そろそろコードレビューやってみようか」と言われたエンジニア
この記事では、一部、弊チームだけのルールもご紹介しております。
そのため、全てのルールがベストプラクティスとは限りません。
是非、ご自身の環境に合った形でカスタマイズしてご活用ください。
結論
下記の順でコードレビューすれば手戻りが少なくなります。
- コードの書き方を見る
- コードの中身を見る
- ローカルで動作確認する
- 検証環境サーバで動作確認する
それでは、それぞれの工程について、解説していきましょう！
1. コードの書き方を見る
初心者にありがちですが、まず動作確認始めるのはやめましょう。
まずは「書き方がなってない」部分の指摘ができないか、から始めましょう。
ルールに従って書かれたコードは読みやすく、メンテナンスしやすいですよね。
ブランチ名が適切に命名されているか
マージリクエスト、マジリクコメントが適切に作成されているか
クラス名、変数名が適切に命名されているか
- 初心者プログラマーのための変数/関数/メソッドの英語命名規則を読みましょう。
テストが記述されているか
ケースが規約通りか
※下記は弊プロジェクトのルールです。
- PHP
- 関数(クラス)名：パスカルケース(アッパーキャメルケース)
- 変数名：キャメルケース
- Model(カラム名)：スネークケース
- Model(定数名)：アッパースネークケース
- Model(定数の配列内のキー名)：スネークケース
- Controller(Propsのキー)：キャメルケース
- JavaScript
- 変数、関数名：キャメルケース
- Propsのキー：キャメルケース
- formのキー：スネークケース
- HTML
- タグ名：ケバブケース
- クラス名：ケバブケース
2. コードの中身を見る
書き方の指摘がなくなったら、コードの中身を見ていきましょう。
「俺だったらこう書くけどね」というコメントも大歓迎です！
不要な記述がないか
- コピペ元の不要な記述が残っていないか
- 検証用の記述(dd()/console.log()など)がないか
コメントが不足していないか
- 初見で分かりにくい場合、コメントが必要な旨、指摘しましょう。
PHP,Laravel,JavaScriptの標準関数を使えないか
記述が重複していないか
- DRY原則に従いましょう。
- ロジックの場合、アクションズクラスに切り出す
- 値の場合
- 単一モデルで使用する場合はModelにて定数化
- 複数モデルで使用する場合はapp/consts配下で定数化
ロジックが記述されている場所は適切か
- Route, Controllerへの記述は極力避け、アクションズクラスを作成しましょう。
メソッドの引数、戻り値の型指定は適切か
メソッドの引数にnullが来る可能性はあるか
- 来る場合の処理が記述されているか要確認です。
バリデーションは適切か
サーバー負荷かかっていないか
- with()忘れてないか
- setAppends()忘れてないか
3. ローカルにチェックアウトして動作確認
コードが大体OKになってきたところで、動作確認しましょう！
仕様書の通りに動作するか
- 動作確認のチェックリストを作りましょう。(本来、レビュイーが作成すべきものですが...)
デグレが起きていないか
- フラグON/OFF両方で動作確認しましょう。
4. 検証環境にデプロイして動作確認
ローカルでの確認が終わったら、サーバー上でも動作確認しましょう。
ローカルでは再現できない不具合を発見できるかもしれません。
実機(PC,iPad)にて表示崩れがないか
- 特にiPadのWebViewで表示する画面は実機で確認しましょう。
指定時間に定期実行されるか
- ローカルでは再現できないので、サーバー上で確認しましょう。
デッドロックが起きないか
- ローカルでは再現できないので、サーバー上で確認しましょう。
おわりに
コードレビューのやり方についてまとめました。
いきなり全部をマスターするのは難しいですが、大体の流れを理解して進めることが大事です。
GOOD LUCK!!!