🖥
ITエンジニア - いつも何を考えてコードレビューをしているのか？ コードレビューの観点を書き出してみる
仕様/実装
- バグがないか？仕様通りに動くか？ ( 「コードだけ」の視野の狭いレビューに陥るべからず。これがいちばん大事な観点である )
- コードレビューだけではなく、動作確認をするのがベスト。
- 要件は満たせているか。「明文化された要件」すなわち「決まったこと」以外の部分において「本来の要求」を満たせているか。 ( クライアントも気づいていない問題・課題が見つかった場合は、それを共有・解決すべきだ )
可読性
- コードの保守性、命名、可読性、分かりやすさ
- 冗長な処理はないか、もしくは、逆に共通化しすぎている処理はないか
- マジックナンバーなどの基本的なアンチパターンがないか。
- コードだけでは処理が分かりにくい部分では、必要なコードコメントがあるかどうか。もしくはコードコメントがなくても分かりやすい命名・構成になっているか。
- コードコメントがある場合、そのコードコメントの適切さ。
- 分割・統合のバランス感は適切かどうか ( ファイル分割、関数の粒度など各所において )
- ファイル名はがエディタで探しやすいものになっているか
- 「どちらでも同じように動くが、より良い書き方」がないか
セキュリティ
- セキュリティ的に脆弱性がないか。セキュリティ的な危険に対してのテストが書けているか。 ( 例: 「 任意のIDを指定すると、他のユーザのデータも見られてしまう」みたいなケースを防ぐテストができているか )
負荷
- 負荷の高い処理が発生していないか ( N+1クエリなど )
- データ量が増えた時に高付加になる処理をしていないか、また、その考慮・確認はできているか
テスト
- 実装に対応した適切なテストが書かれているか
- テストの管理コスト、テストの可読性、品質保証のレベル
- テストが仕様書的に読めるかどうか
データベース
- データベース設計、正規化・非正規化の度合い
流儀
- 言語、フレームワークの流儀に則った書き方をしているか
- 既存のコードとの統一感。「見慣れない書き方」「浮いてる書き方」をしていないかどうか。
エラー / バリデーション
- エラーハンドリングは適切にできているか、エラーメッセージは分かりやすく適切かどうか
- アプリケーションのロジックによるバリデーションだけではなく、データベース層での制約をつけているか
- フロントエンドだけのバリデーションだけではなく、バックエンドのバリデーションができているか
Pull Request / レビュワーに対して
- 「レビュワーに優しいか」どうか
- 動作確認の例、スクリーンショットなどがDescriptionに載っているか
- PRの単位は適切かどうか ( 差し戻し = Revert をした時に、意図しない変更まで巻き戻ってしまわないかどうか )
コミュニケーション
- レビューのコストパフォーマンスを考える
- レビューコメントが往復する場合や、話が複雑になる場合はZoomなどで話す
チャットメンバー募集
何か質問、悩み事、相談などあればLINEオープンチャットもご利用ください。
プロフィール・経歴
公開日時
2024-11-16
Discussion