概要
チーム開発しているとコードを書く以外に他のエンジニアのコードレビューをする事ももちろんあるわけで、その時自分は何を見て何を考えながらレビューを行っているのかが言語化できていなかったので言語化する。
何を見ているのか
タイポがあるか
命名規則の一貫性があるか
例えばキャメルケースとスネークケースが混在していないか
変数や関数、interface などの命名は適切か
パッと見て何が代入される変数か、どんな振る舞いのある関数なのかが認知できるかを見る
自分が100%正しいとは思っていないし、そもそも勉強中なので基本は提案ベースで確認する。
命名については他にもメンバーがいれば「自分はこう思うけど◯◯さんはどう思います？」みたいな感じで他の人にも見てもらうようにする
命名と振る舞いが乖離している場合は振る舞いに命名を寄せてもらうよう提案する。
関数がファットになっていないか
一つの関数で色んな事をやっている場合は上手いこと分離できないかを自分自身でブランチをクローンしてきて手元で試行錯誤する。結果を参考コードとしてコメントで提案する。
自身が未熟故に思い浮かばないときは、雑に分離できないかを聞いてみる。
繰り返し系の構文がネストされている場合も分離できないかを検討する
冗長な書き方があれば簡潔な書き方にできないか
一つのPRに同じコードが散見される場合は、共通化できないか
コードに適切なコメントが記載されてあるか
何をもって「適切」かという判断が難しいところだが、パッと見て7~10行近くあるコードであれば何をしているコードなのかはコメントを記載してもらえないか確認してみる
プロジェクトのデザインパターンに沿った書き方や作りになっているか
例えば新規でディレクトリが作られた場合とかは既存のところにかけないかどうか検討する
何を考えているか
コメントする場合に伝え方がトゲトゲしくなっていないか
嫌味な書き方になっていないかなど
コメントをする場合、自分のコメントが相手に正しく伝わる、理解できる書き方になっているか
自分がコードを書くならどう書くか
プルリクの概要に記載されている（またはプルリクの元となったissue）のスコープが守られているか
スコープ外の対応がある場合は別途issueを立ててもらってそちらで直してもらうか、プルリクの概要にスコープ外の対応として記載してもらう
これはコードレビューというよりはプルリク自体の話になっているかも
実装してくれた人への感謝とリスペクトを忘れない
まとめ
下記のスライドに戦略的コードレビューと戦術的コードレビューなる言葉が出てきているが、自分は主に戦術的コードレビューを行っているように感じた。
ただ、後で直したほうが良さそうな部分はFIXMEとか残しておいてもらって、別途issue立ててもらって（自分が立てることもある）、今度隙見て直しましょうか〜みたいな流れにする。
おわり。