- チェックリスト - レビューでチェックすべき項目のリスト
- はじめに
- 不具合修正の場合のチェックリスト
- 新しい機能を追加する場合のチェックリスト
- 共通のチェックリスト
- チェックリストの補足: セキュリティおよび性能
- 2019-04-12 ごろ作成
不具合修正の場合のチェックリストと、新しい機能を追加する場合のチェックリストを作りました。 2つに共通する項目は、「共通」の節にまとめました。
チェックリストは reviewer がレビューをするときに使うように書きましが、reviewee がレビュー提出前に自分でチェックするのにも使えます。
チェックリストは、大項目(インデントされてない項目)だけチェックすれば良いように作りました。 サブ項目(インデントされている項目)は、それがぶら下がっている大項目を確かめる具体的な方法の説明です。
セキュリティや性能に関するチェックリストは、補足として別の節に分けました。
- 不具合の発生理由は明らかになっていますか？
- その不具合が起きた原因は何でしたか？
- その不具合の原因を引き起こした原因は何でしたか？
- 修正コードがないと、報告された不具合が発生し、失敗するテストコードになっていますか？
- そのテストコードはどのファイルの何行目にありますか？
- そのテストコードはどんな機能をどのように確かめていますか？
- 不具合を修正するコードが作成されていますか？
- 上で明らかになった原因はどのように解決されていますか？
- それはどのファイルの何行目ですか？
- 仕様が実現していることを確かめるテストコードは作られていますか？
- そのテストコードは、機能追加をするコードがないと失敗しますか？
- そのテストコードはどんな機能をどのように確かめていますか？
- そのテストコードはどのファイルの何行目にありますか？
- 新しい機能を追加するコードは作成されていますか？
- 新しい機能を追加することは、どのように実現されていますか？
- それはどのファイルの何行目ですか？
- テストコードを走らせたら error や failure は起きませんか？
- 開発環境でブラウザから動作確認し、意図したとおりの動作になっていることを確かめましたか？
- ブラウザから動作確認するとき、どのページでどんな操作をして動作確認をしましたか？
- 動作確認をする手順は、他の人も同じように操作して再現できるほど明らかになっていますか？
- テスト用データは用意されていますか？またはテスト用データの作成方法は明らかになっていますか？
- コーディングスタイルは良さそうですか？
- SQL インジェクション
- 文字列の連結で SQL 文を組み立てていませんか？
- SQL 文中で文字列変数を展開していませんか？ 例: "where column = #{variable}"
- XSS, CSRF
- 安全でない文字列をブラウザに返していませんか？
- 権限チェック: ログイン済みユーザの権限で許される操作かどうかなど
- 外部から来るデータ params の取り扱い
- 内容を精査せず、そのままモデルに mass assignment していませんか？
- アップロードされたファイルの取り扱い
- ファイル名に .exe などの拡張子がついていても、安全に取り扱えますか？ もしくは排除できますか？
- 悪意ある内容が含まれたファイルがアップロードされても、安全に取り扱えますか？ もしくは排除できますか？
- 不意に大量のデータが送られてきても安全なコードになっていますか？
- ブラウザへの応答時間は、なるべく0.5秒以内にします。1秒を超える場合は遅延ジョブなどの利用を検討します。
- データ量が数件、数十件合、数百件、数千件と増えていくとすると、処理に要する時間はどのように増えていくかを考慮します。
- データ量に対し、処理に要する時間が比例して増えていく場合、つまり O(n) の場合は注意が必要です。
- DB でインデックスが効いている場合の検索や、ハッシュの要素へのアクセスは O(log n) になったりするので比較的安全です。
- 計算量については下記のページをご覧ください。
- レビュー対象のコードで発生したり処理したりするデータ量の大まかな見積もりをします。
- 一般的なディスク書き込み速度やネットワーク速度は、おおよそ数十 MB/s です。
- つまり、取り扱うデータ量が数メガバイトから 数十メガバイトになると、ブラウザへの応答速度が0.5秒を超える恐れがあります。
- ですので、取り扱うデータ量はなるべく数メガバイトを下回るようにします。