社内勉強会(?)の資料
はじめに
コードレビュー何だかうまくいかない
[Googleエンジニアリング・プラクティス](https://shuuji3.xyz/eng-practices/) を参考にレビューを改善しよう
レビューの目的
コード全体の健全性が、改善されている（≒コードがいい感じになる）ことを検証する
要件を満たしている
バグがない
複雑でない
仕様変更や拡張に耐えられそう
approve の基準(LGTMの基準)
そのプルリクエストがマージされたら、コードの健全性が改善される
悪化してなければ OK
完璧じゃなくても OK
改善サイクルを回す
言うべきかそうじゃないか問題
考えなくていいこと
単なる自分の好みを押し付けていないだろうか？
コードを批判すると author を傷つけてしまうのでは？
コードの健全性に影響あるかを考えよう
よくなる→言うべき
そうでもない→言ってもいいし言わなくてもいい
定型文に当てはめたら楽
提案＋理由
X にするのはどうですか？ なぜなら Y だからです
Y だから X にするのが良いと思います
時間がない問題
それでも素早くレスポンスしよう
プルリクの author はレビューを待っている間に、次のタスクを進めることもできるが切り替えコストが発生する
マージできない・返事がないストレス
考慮漏れなどが手遅れにならないように
チームの生産性低下
とはいえ無理はしないで
自分の生産性も大事
プログラミングに集中している時間というのは非常に貴重
一旦集中が切れると、帰ってくるのに30分くらいかかるらしい
妥協点：レビューするタイミングを決める
朝出社してプログラミングを始める前の時間
トイレに立ったあと
休憩が終わったあと
会議のあと
そもそもプルリクの内容がわからない問題
プルリクで何してるのか不明
他の人にお願いして自分は辞退する
Description や関連する Issue を見る
プルリクエストをチェックアウトして手元で動かしてみる
コードが複雑すぎてわからない
モデリングとか何やらかんやらで改善できないか相談
教えてもらう → 極力コードに反映するようにお願いする
レビュー出す側にやってほしいこと
プルリクは小さくして欲しい
大きいと見るのにとても時間がかかる
時間がかかるとコンフリクトしやすくなる、コンテキストスイッチも増える
5ファイル/300行くらいが理想、新機能を作るときでもその倍に収まると読みやすい
履歴を整理して欲しい
rebase -i を使ってコミット順並べ替えたりマージしたり
typo などは歴史を残す必要がないのでコミット改ざんして良い
その他のトピック
興味があれば [Googleエンジニアリング・プラクティス](https://shuuji3.xyz/eng-practices/) 読んでみましょう。
プルリクのどこから見るか？
コメントの文章の書きかた
提案に反対されたときどうするか
コードレビューの観点
コードがよく設計されている。
機能がコードの利用者にとってもよいものとなっている。
すべての UI の変更は、意味があってよいものに見える。
並列プログラミングでは、すべての処理を安全に完了させている。
コードが必要以上に複雑ではない。
開発者が、現時点で必要かどうかは分からないが、将来必要になるかもしれないものを実装していない。
コードに適切なユニットテストが存在する。
テストがよく設計されている。
開発者がすべてのものに明確な名前を使用している。
コメントが明確で役に立つものであり、大部分はコードの動作 (what) ではなくそのコードである理由 (why) を説明するものとなっている。
コードに適切なドキュメントが書かれている。
コードが Rubocop などのスタイルガイドに従っている。
すべての行をみた？
コンテキストを考えた？
よいことについてコメントした？
レビュー受ける側はどうするのか
さいごに
初心者でもレビュー参加していこう
最初はたくさん時間がかかるけれど、みんなに経験が積み上がっていけば自ずと生産性があがっていくはず
自分がわからない = ほとんど皆わからない ので気楽に質問/相談しよう