レビューする人も、される人も知っておきたい！開発プロセスごとのレビュー観点チェックリスト

こんにちは。リンクアンドモチベーション、モチベーションクラウド開発チームの江上です。

モチベーションクラウドの開発チームは、新卒メンバーも多数参加していることもあり、開発プロセスを細かめに定義しています。その開発プロセスにおいてそれぞれに、「レビュー観点チェックリスト」を作成しているので、それを公開していきたいと思います。
全てのチームで有効とはいかないものの、何かの気付きになればいいなと思います。また、「こういう観点もあるよね」というFBは大歓迎ですのでぜひよろしくお願いします。

開発プロセスの定義

現状の開発プロセス

現在モチベーションクラウドの開発チームは以下のようなプロセスを前提にして動いています。
関係者レビューは実際には実装のレビューに近しいですが、実装レビューはコードレビューと紐付けたいので、あえて別で記載しています。

要求整理
要件定義
一次計画
基本設計
二次計画
詳細設計
実装
関係者レビュー
1. デザイナーレビュー
2. プロダクトマネージャーレビュー
3. ユーザーサポートレビュー
テスト設計
テスト実施

以降、上記の区切りを前提として、それぞれのプロセスにおけるチェックリストを紹介していきます。

1. 要求整理

要求整理は顧客の声や、同僚の意見から「なぜそう思っているのか？本当は何がしたいと思っているのか？」を言語化するフェーズです。

要求整理のレビュー観点チェックリスト

要求になってるか？
　顧客の気持ちや背景をちゃんと記載できているか？

　「〜したい」という語尾で統一されているか？
　顧客が実際に言っていることなどの生の意見が記載されているか？（集約したexcelへのリンクでも可）
　「なぜそういう意見がでたのか？」を言及できているか？
　生の意見を集約・抽象化する形で、「要求」が記載できているか？

要件になってないか？
　具体的な解決方法を記載してしまってないか？

　「xxxをできるようにする」と記載してないか？
　「xxxの項目を追加する」と記載してないか？

2. 要件定義

要件定義は要求整理で得られた要求を実現するために、「何をしなければならないか？」を言語化するフェーズです。

要求との接続
　要求に基づいて、それを実現するために要件が作られているか？確認します

　「〜できること」という語尾で統一されているか？
　要件に紐づく要求との対応関係が明確か？

スコープ
　開発のスコープ（=変更すべき点）が明確であるか？確認します

　実現可能か？
　変更すべき点がわかるように記載できてるか？
　影響範囲はコードレベルでざっくり確認できてるか？（関連ワードgrepするくらいでよい）
　関係しそうだが、変更してはいけない点に関して記載できてるか？
　デザインが確認できるか？
　非機能要件は定義されてるか？
　異常系・準異常系の挙動も定義できてるか？

アウトカム
　要求を実現できたかどうかを計測できる状態にあるか？確認します

　機能リリース後に参照したい値が決まっているか？
　機能リリース後に参照したい値の目標値と期間が決まっているか？
　参照したい値がまだ計測できてない値である場合、スコープに計測追加も含まれているか？

3. 一次計画

一時計画は、ざっくり計画を立てて体制やだいたいのリリース時期を決めるフェーズです。

計画のレビュー観点チェックリスト

全体

　うまく行かないとしたら原因は何か？
　完了条件は何か？
　他の選択肢はないか？

縦軸（項目）

　外部への協力・依頼etcがタスクに入っているか？
　QAのスケジュールも引けているか？
　QAで非機能テストのタスクが入っているか？
　バッファが適切に設けられているか？

横軸（線）

　可能な限り機能を分割してリリースする計画になっているか？
　不確実性が高い箇所はどこか？
　不確実性が高い部分を先にやれているか？
　並行するものが多すぎないか？

4. 基本設計

データ定義やアーキテクチャなど基本的な方針を決めるフェーズです

基本設計のレビュー観点チェックリスト

方針

　要件を満たしているか
　概念が混在していないか
　要件を満たす正しい概念は何かを明示する
　再度要件に立ち返って、概念の分離や要件の訂正を行う
　refactorラベルがついてるstoryで一緒に解決しちゃえるものはないか？
　インシデント一覧を確認し、再発しそうなものがないか

バックエンド

　APIに漏れがないか？（フロントと認識あってるか？）
　API設計がガイドラインに違反していないか
　データに漏れがないか？
　データの流れに漏れがないか？
　影響範囲に漏れがないが？
　ネーミングは適切か？
　データが機密情報に該当しないか？　するなら暗号化や開発環境でのマスキングが考慮されてるか？

フロント

　一回の処理が重くないか？するなら、分割できないか？
　細かすぎるものを統合できないか？
　フロントで計算できるものがないか？
　ネーミングは適切か？
　フロントでデータを保持する必要意味があるか？　あるなら、ストアが適切か？

その他

　要件・設計書において、米印がつくような注意事項に関しては抜けやすいのでspecを書くことが予定されているか？

5. 二次計画

設計が決まったのでチケットまで細分化し見積もりを行うフェーズです

計画のレビュー観点チェックリスト

一次計画レビューと同じため省略

6. 詳細設計

コードレベルで、どういうフォルダ構造にして、どういう名前のどういうインターフェースをもつクラスをつくるかなど基本設計をもとに記載していきます。

詳細設計のレビュー観点チェックリスト

ファイルシステム

　フォルダ構造が適切か？
　prefix、suffixの命名ルールが統一されているか？
　このフォルダにファイルを足したいとき、どこにどういう名前で足せばいいかの予想がつくか？

インターフェース

　採用しているデザインパターンの予想がつくインターフェースになっているか？
　責任範囲が明確な名前になっているか？
　そのクラス、もしくはインスタンスが主語である前提でインターフェースの名前が定義されてるか？（受動態、能動態があってるか？）
　処理を追加したいとき、どこに書くべきか明確か？

7. 実装

実際にコードを記述するフェーズです。
ここでは、コードレビューの観点を示します。コード部分なので、Ruby on Railsや特定のGemに依存した観点も一部ありますが、そのまま記載しています。

コードレビューの観点チェックリスト

プルリク

　PRの粒度が適切か？（機能ごとにPRが分けられているか？）
　コメントで変更点だけではなく、変更理由も記載されているか？

名前

　わかりやすい名前になってるか？
　マジックナンバーがないか？（例: 特別な意味を持つ数字や文字列は定数で定義しているか）
　同じものには同じ名前が使われているか？

データの検証

　変数がnilだったとき落ちるようなコードになってないか？
　selectやupdateするときに対象としているstatusは正しいか？
　最新のレコードを取得する時に参照するカラムが正しいか？

データベース処理

　n + 1が発生していないか？
　適切なトランザクションが張られているか？
　ロックが適切か？（ちゃんと開放するか？不用意な範囲ロックを取得してないか？）

コメント

　メソッドの内容ではなく、意図を記載できてるか？

システム効率

　容量やレコードが多いテーブルとjoinしてないか？
　ループ内での初期化や無駄なクエリ発行がないか？
　Array#find or Array#select etc..で配列内をループしても、問題ないデータ母数か？
　map(&:id) => pluck(:id)に書き換えられないか？

ミスしにくい書き方

　importメソッドにて、理由がない限り、on duplicate key updateはfalseで指定されてるか？
　transaction内で非同期処理の呼び出し（perform_later)などをしていないか？
　reloadではなくfindが使われてるか？（reloadは原則禁止！）

テスト

　rspecのdiff coverageが100%となっているか？

セキュリティ

　action単位で認可処理が問題ないか

エラー

　エラーを握りつぶしてないか(正当な理由がない限り)

全体最終確認

　消えているべきメソッドが消えているか？
　コードや名前に一貫性があるか？

8. 関係者レビュー

実装されたものを関係者で確認するフェーズです
関係者レビュー観点はまだあまり充実してないので、充実してきたらアップデートしようと思います

9. テスト設計

実装されたものをどうテストするかを設計するフェーズです。

テスト設計の観点チェックリスト

仕様

　機能概要の説明に過不足がないか？
　操作者のロールによる機能制限はないか？
　操作者のカスタム権限による機能制限はないか？
　画面を見て、自分が知る範囲・想像できる範囲の仕様でチェックが漏れてそうなところないか？
　前提となる設定が変更または削除された時の挙動チェックがあるか？

他機能への影響

　該当機能でwriteしたものを他で参照している機能がうごくことをチェックする項目があるか？
　「5分適当に他機能も含め触る（モンキーテスト）」という項目があるか？

外部サービス

　メール送信のテスト時は、メール受信まで確認できることを確認しているか？

検証パターン

　画面表示で、「リロード、ブラウザバック、直リンクでの表示チェック」の検証できてるか？
　境界値で検証できてるか？（pagingの1ページの数に応じて、1page, 2page, 3page。日付の月またぎ、年またぎ。入力文字数（0,1,255,256文字など（255文字の場合）など）
　異常値で検証できてるか？（特殊記号（jisではあるがutf8でない文字）や空白、大文字/小文字、非表示文字、絵文字、機種依存文字、”〜”）
　セキュリティ的な入力値が検証できてるか？（scriptタグやinjection）

その他