norenの担当しているヤマグチです。
変なリリースをして止めた経験を元にレビュー時の着目点として出てきたものをまとめたものとなります。
そのためより、見る人から見たら足りない物かとおもいますがこの内容でも充分に効果がありました。
目的
・変なリリースをしてサービスを止めないこと
コードレビュー着目点(Symfony2)
・ワークフローに則った修正か
□ PullRequest先（merge先）は正しいか
→Github-flowなのに旧にmasterにPullRequest来てないこと等
・変更量
□ 当たり前だが量が多いほど注意すべき
・動作確認
□ 開発環境で動作確認済みか
□ テストが通っているか
※これが済んでいれば完璧だが必ずしもテストが済んでいるとは限らないため
・影響範囲の推定
□ Configに対する修正か
→要確認：デプロイ後きちんと反映されているか
□ Entity(Model)に対する修正か
→仕様変更(修正の要件)にあった修正か
→Entitiyの名前やカラム名でGrepして影響範囲の全てが修正できているか
・また想定内か
□ Controllerに対する修正か
→Error処理は仕様通りか
→外部IF仕様は変わっていないか
□ Viewに対する修正か
→CMS等の直接使用者の目に触れているところならば、開発者以外のレビューは済んでいるか
・開発者が使いやすく、消費者などの使用者が使いにくいUIになっているかもしれないため
□ ミドルウェアが関係する修正か
→要テストと修正内容の把握
□ AWSに関する修正か
→S3、EC2、VPC、RDSなんであっても要テストと要計画
・コーディング規約をまもった書き方か確認
□ チーム内で統一された書き方となっているか
→大きく外れた書き方をしている場合は要修正
・リリース後の問題発生時に対応しずらくなるため。（そのため細かい点は許容してもいいかと）
□ コメントはきちんといれてあるか
→自分が関わったところはいつでもコードを読めば説明できる程度にいれておく
・仕様変更点の確認
□ 変更範囲を超えた修正が無いか
・影響範囲を推定した結果と照らし合わせていけば越えているかどうかが分かる
□ テストが本変更に対して理にかなっているものとなっているか
・類似の修正が無いか確認
□ 今回の修正で類似の問題も治るか
・治らなければ忘れぬようにする
→きちんと理由もメモっておくこと
以上です。
最低限この内容だけでもチェックしておけば、悲惨なことにはならないかと思います！
エンジニア：山口恵輔
一覧に戻る