- 誰かだけが触れるコードを無くす
- 自分だけが持っているコードを無くす
- 自分だけが触っている時間を短くする
コードレビューは...
- 相互学習型のプロセスである
- メンバが成長することが大事
- レビュアーとレビュイーが、お互い学び合うこと
- 考え方を共有すること
- 質問することで学ぼう
- 一番できる誰かだけが教えるのではない
- 知識や経験の少ない人が何に躓いているのか学ぼう
同じミスをチーム内で繰り返さないことが成長
- 本人の問題にしない
- 明日はわが身
- 仕事の正しい手順を覚えよう
- 道具の正しい使い方を覚えよう
- 伝えることが大事
- 改善するまでがレビュー
- レビューにコストをかけ過ぎない
- レビュアーはレビュイーに伝わるように努力しよう
- できる限り丁寧な表現を心がける
- 同じ表現で伝わらないなら言い方を変える
- 感銘を受けたり前回から改善した部分を褒めよう
- 厳しい事しか言わない人の話を聞き続けるのは難しい
- 成長に気が付いて貰えると嬉しい
- 指摘して終わりではなく改善したら終わり
- 指摘事項の改善を確認するのはレビュアーの責任
- レビュアーがかける時間
- レビュアーの人数
- レビュイーの改善にかける時間
- レビュアーとレビュイーのメンタル
- レビューは改善プロセスであって成果物そのものではない
- レビューだけやっていても仕事は終わらない
- レビューはコードを書いてる時間の半分以下を目安に
- お互い丁寧に説明し過ぎない
- 毎回全員が参加する必要はない
- 完全なものではなく、現時点で妥当なものを選ぶ
- レビューはメンタルを消耗し易い
- 特にレビュイーは消耗し易い
- 被疑者と取調官になってはいけない
- 仕様把握
- 下読み
- エラー、ワーニング確認
- 仕様との整合性確認
- プロジェクト標準との適合性確認
- レビュー会
- レビュー対象となるコードの機能仕様を理解する
- 仕様を把握せずにレビューするとコードの非常に細かい部分の指摘が増える
- レビュー対象となるコードを全て読む
- レビュー対象を呼出しているコードも読む
- レビュー対象が呼出しているコードも読む
- コンパイラや静的解析ツールのエラーやワーニングが無い事を確認する
- ワーニングをSuppressするのは問題ない
- ワーニングをSuppressしたら理由を説明すること
- レビュー対象のテストが成功することを確認する
- ビルドできるコードだけをレビューすること
CIが回っていれば自動化できる
- 仕様通りに動作するコードなのか確認する
- 自然言語で書かれた仕様書とコードをできる限り一致させる
- 順序や数、名前など
例えば、以下のような観点で確認するとよい
- 受け入れるパラメータ
- 定義順序と数
- 値の範囲
- 処理結果として返す値の範囲
- 処理順序
- 分岐の評価順序
- エラー処理
- 呼出し先に渡すパラメータの範囲
- プロジェクトメンバ全員が同じようにコードを書こう
- 自分達が納得できるやり方でやろう
- 遠くの偉い人が言ってる事に従ってるだけではダメ
- まずはeclipseのコードフォーマッタを共有しよう
- インデント、括弧の位置、等を揃えると読み易くなる
これはあくまでも例であって、自分達に合うやり方を探す事
- 変数名の長さとスコープの広さを近似する
- メンバ変数名 > パラメータ名 > ローカル変数名
- メソッドは一画面に収まるように書く
- ローカル変数はできるだけ少なく
- パラメータとローカル変数を併せて7つ程度に収める
- if文で!（エクスクラメーション）を使わない
- !は特別なフォントを使わない限り見落とし易いため
if(condition == false)と書く
- 比較演算子は
<と==だけを使う - 例外オブジェクトをなるべく避ける
- 基本的にはオンラインでやろう
- 会議室に集まるのは...
- ニュアンスが難しい事を指摘するため
- オンラインでの議論が収束しないとき
- ソースコードはプロジェクトの共同所有物
- レビューを通して成長しよう
このスライドはMarpで作りました