Skip to content

Instantly share code, notes, and snippets.

@tai2
Created September 8, 2022 07:09
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
Star You must be signed in to star a gist
Save tai2/bba4e5c394415d54e271b91d2bec3651 to your computer and use it in GitHub Desktop.
プルリクエストレビューチェックリスト
  • 全体的な意図。なにをどうやって実現するのか(What, How)。
  • 変更の文脈、位置付け。なぜこの変更が必要なのか(Why)。
  • 複数の独立した問題が含まれているか
    • 含まれている場合、分割できないか検討する
  • 設計の誤りは影響が後を引く可能性があるので、なるべくちゃんと見ておきたい。とくに永続化されるデータ構造のミスは、リリースしてしまうと修正な面倒なので、注意する必要がある。例えば:
    • SQLアンチパターンに該当するようなテーブル設計になっている(実データが発生するとめんどう)
    • 本来別のAPIを新設すべきところを、既存APIへの追加パラメータで無理矢理処理している(修正される挙動への依存が増えるとめんどう)
    • 手続の種類が増えても修正不要なように書ける(一度書けば済む)のに、手続の数だけコード追加が必要な設計になっている(無駄な手数が増える)
  • 追加・変更される挙動について、テストケースが追加・変更されているか、テストケースの見出しレベルで簡単に見る
  • 特殊な理由のある変更など、コードだけから理解できなさそうな変更は、コメントに経緯が書かれているかを見る
  • UIの変更が含まれる場合、スクリーンショットが添付されているか
  • QA手順が記載されているか
  • これ以外のすべて: セキュリティー、局所的なコード品質などは最悪見なくてもいい
    • 効率よくレビューしつつ、セキュリティーをいかに担保するのかという点については、いまのところどうすればいいのかわからない。時間をかけずに効率よくレビューするという命題と、セキュリティー検査をしっかり行うという命題は相容れない気がする。
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment