=================================== 43:レビューのチェックリストを作ろう =================================== .. maigo:: どこまで確認したの？ * 後輩W：このPRをレビューお願いします（レビュー観点を自分でも確認したから、今度こそバッチりだぞ！） * 先輩T：はい（1分後）うーん、だいたいは良いと思うんだけど、規約に合っていない部分がいくつかあるみたいだね。セルフレビューで確認してる？ * 後輩W：もちろんです。どこが合ってないですか？ * 先輩T：このファイルなんだけど、これだとログに個人情報が出てしまうんじゃないかな？ * 後輩W：あっ、ほんとうだ。おかしいな、ちゃんとレビュー観点を元に確認したんですが、見逃したみたいです。すみません。 * 先輩T：なるほど。どこまで確認したか、ちょっとまとめて教えてくれる？ レビュー観点が用意されていてその観点を確認していたとしても、どこまで確認したかが不明確だとレビューアーは効率良くレビューを進められません。 それに、確認項目の見逃しはどんなに開発に慣れてきても発生してしまうものです。 記憶や慣れに頼らずに、漏れなく確認する方法を検討しましょう。 ベストプラクティス ================== :index:`レビューチェックリスト` を作っておき、レビューを依頼する前にチェックしましょう。 チェックリストがあればチェック漏れをなくせますし、レビュー依頼される人も「ここまでは自分でも確認しているんだな」ということがわかります。 GitHubであれば、PRのテンプレートを用意できるので、以下のようにチェック項目として観点を記載しておきましょう。 レビュー依頼前のチェック項目があれば、「先に言ってよ」という問題も回避できます。 .. index:: PULL_REQUEST_TEMPLATE.md .. code-block:: md :caption: ``.github/PULL_REQUEST_TEMPLATE.md`` PR作成時のチェック項目 - [ ] チケットタイトルを次の書式で記載したか？ `refs # <チケット名>` - [ ] labelに `WIP` を指定したか?（レビューが必要になるまで付けておく） - [ ] labelの `WIP` を解除したか?（レビューが必要になったら） - [ ] reviewersにレビューアーを指定したか？ チケットURL - [ ] https://github.com///issues/99999999 このレビューで確認してほしい点 - [ ] 機能xxxをクリックしたらxxxxできること <仕様1リンク> - [ ] 機能yyyをクリックしたらxxxxできること <仕様2リンク> レビュー提出前 規約セルフチェック - [ ] C1 各種機能に適切なパーミッションが設定されているか - [ ] C2 変更が発生するリクエストではCSRFトークンを使用しているか - [ ] C3 トークンは適切な時間で破棄されているか - [ ] C4 エラーログ、スタックトレースに重要情報が含まれていないか - [ ] C5 /tmp にファイルを書いていないか - [ ] C6 SQLを文字列操作で組み立てていないか - [ ] C7 システム外部から渡ってくる入力はバリデーションしているか - [ ] D1 モデルの構造に着目したチェック - [ ] D2 機能単体に着目したチェック - [ ] D3 機能の結合に関連したチェック - [ ] E1 処理の長さで関数を分割しない - [ ] E2 引数の数を減らす - [ ] E3 継承の利用を最小限にする(Flat is better than nested) - [ ] E4 継承で挙動を変えていないこと(リスコフの置換原則) - [ ] E5 型ヒントが書かれてること - [ ] E6 ログ出力は規約に合っていること - [ ] E7 実装されている変更は仕様(Wiki)に記載、反映されていること レビュー提出前 動作セルフチェック - [ ] UnitTestはすべて通っているか - [ ] 差分は期待どおりに動作しているか レビューアーからの確認項目 - [ ] <確認内容> - [ ] <確認内容> 関連 ======= * :doc:`../レビュー/42-レビューアーはレビューの根拠を明示しよう`