はじめてのPHPプロフェッショナル開発 ch12 Pull Request駆動によるコードレビュー
コードレビューの必要性
- コードの品質を高める
- バグのないコードを書き続けるのは不可能
- 思い込みや先入観のない目で見直してもらう
- バグのないコードを書き続けるのは不可能
- 属人性の低減
- コードを実装者のものからチームのものにする
- 心理的安全性の向上
- 不具合発生時の責任感もみんなで背負う
- 個々人の技術力の向上
- 他人のコードを読むことでも技術は向上する
- レビュアーとレビュイーとでコミュニケーションをとりながら切磋琢磨
Pull Requestを利用したコードレビュー
- 前職ではコードを印刷してレビューしてました 終わっている
Pull Requestとは？
- GitHub, GitLab, BitBucket等だいたいおなじ(名前はちがう)
- 「僕がつくったブランチは問題がないのでmasterブランチにマージしてください」とお願いする作業のこと
GitHub Flow
- remoteからリポジトリをcloneする
- masterブランチからfeatureブランチを切る
- featureブランチ上でコードの改修を行う(適宜commit)
- 改修が一通り完了したらremoteにpushする
- 問題がなければmasterブランチにmergeする
- masterブランチのコードをデプロイする
Git Flow
- Git Flow
git flowコマンドを使えるように、拡張を入れたりするらしい
Pull Requestをつくってみよう
- GitHubの場合、コミットメッセージの内容からPRをつくれる
- 1行目: title
- 3行目以降: description
- md記法使える
Pull Requestの作り方
title
fixed #123とか書くと、マージされた時対応するissueを閉じてくれる- 途中でレビューしてほしいときは
WIPとか明記するとよい
description
- やったこと
- 対応するissueへのリンク
- レビュアーの無駄な指摘を減らすために下記もやると親切
- やっていないこと
- hotfixのときは、「現時点では妥協している点」とかも
- 一次対応後に別途改善が必要なら、後から再度PR送る
inline comment
- コードに行単位でコメントできる
- 妥協点等、レビュアーに突っ込まれそうな点を予めコメントしておくなど
- 無駄な指摘を減らす
- 迷った点等、特に意見がほしい箇所にもコメント
- 【所感】「全部見てください」は経験上かなり苦痛だし集中できない
Pull Requestのコードレビューの流れ
| 実装者 | レビュアー | |
|---|---|---|
| 1. | PRつくる | |
| 2. | コードレビュー依頼する | |
| 3. | コードレビュー行う | |
| 4. | FB | |
| 5. | FB受けて適宜直す | |
| 6. | 2-5繰り返す | |
| 7. | 問題がなければLGTM出す(Looks Good To Me) | |
| 8. | masterブランチにマージする |
- with nits (nitpick), imo (in my opinion)
- 些細なポイントで気になる点があるので適宜修正してください
- 再レビューは不要です
- masterへのマージは実装者がやる現場が多いらしい
- 2019/3現在携わっているプロジェクトはレビュアーがマージしている
コードレビューをしよう
コードレビューの観点
- 再優先事項: ユーザーに迷惑をかけない
- 不具合がないこと
- 要求仕様を満たしていること
- 動的型付け特有の挙動
- 比較演算子
- 識別子名の一貫性
- 無駄に複雑なコードを書かない
- ifのネストとか。本当に必要か
- メソッドの切り出し
- 名前をつけて単一責務に
- パフォーマンス
- 「0.1秒高速化につき売上1%上昇」 by Amazon
N+1のクエリとかはNG
- フレームワークに乗っかる
Collectionクラスを使うとか
レビュアーとして気をつけること
- 人格否定をしない
- 心理的安全性
- 敬意
- 良いと思った箇所を褒めるのもよい
- 客観的に
- 「なんか気持ち悪いコード」とかは駄目
実装者として気をつけること
- 真摯に受け止める
- コミットメッセージをきれいに書く
- コミットは適切な粒度で
- 何をやったのか
- なぜやったのか
