背景・動機
ソースコードレビューをする(受ける)際に、コメントに困る(困らせてしまう)事があります。
自分なりに原因を考え、レビューで気をつけていることをまとめることで、他の人の参考になればと思い投稿します。
レビューするときに気をつけていること
- 具体的に書く
- 理由を書く
- 代案を提示する
1. 具体的に書く
// Bad
もっと良くできるはずなので、頑張りましょう。
// Good
if-elseの階層が深くなっているので、早期returnしましょう。
具体的でないコメントはレビューイを混乱させてしまいます。
そういった意図がなくても批判と受け止められることもあり、レビューに対する意欲の低下にもつながります。
2. 理由を書く
// Bad
変数 user_data の定義を L199 に移動しましょう。
// Good
変数 user_data は利用する直前の L199 で定義しましょう。変数のスコープが広くなると、バグが混入する原因となってしまう可能性があります。
レビュアーは完璧ではないので、間違った指摘をすることもあります。
上記の例で、もしかしたら他の行で user_data が利用されているかもしれません。
コメントに理由が書かれていると、レビューイは指摘の意図を汲んで、変数のスコープをなるべく狭くするよう修正するでしょう。
さらに、次から同様の指摘を受けないよう、学習し改善してくれるかもしれません。
3. 代案を提示する
// Bad
User.checkStatus というメソッド名では、何をチェックしているのか分かりません。
// Good
User.checkStatus というメソッド名では、何をチェックしているのか分からないので User.isActiveState というメソッド名にしましょう。
代案が提示されていないと、レビューイはどう修正すれば分からずコメントを返すか、間違った理解をして修正をしてしまい、コメントのやり取りの回数が増えます。
スケジュールが差し迫っている場合(時間が余っている事などないと思いますが)は、レビュアー、レビューイ共に負担となってしまいます。
上記の例で、レビューイは User.checkUserStatus と修正するかもしれません。
実は checkStatus は Active 以外も判定している可能性もあります。
コメントに代案が提示されていると、レビューイは意図を汲んで説明的なメソッド名を付けるでしょう。
レビューを受けるときに気をつけていること
- 感謝する
- まず最初に自分がレビューする
- 何をレビューしてもらいたいのか書く
1. 感謝する
レビューしてくれたことに感謝します。
例えレビュアーから痛烈なコメントを受けたとしても、時間を割いて自分の書いたコードを読んでくれたことに感謝しましょう。
2. まず最初に自分がレビューする
自分が書いたコードの最初のレビュアーになりましょう。
一旦冷静になって、自分の書いたコードを読み返してみると、良くない部分が見つかることもあります。
3. 何をレビューしてもらいたいのか書く
どういった部分に対して重点的にレビューしてもらいたいのか、自分の意見で良いので Pull Request に記載します。
レビュアーが忙しく時間がない場合(多分そういった人がレビュアーである事が多いと思います)重要な情報源となります。
終わりに
過去レビューにおいて、相手への配慮を欠き失敗したことがあります。
本記事は自分の経験に基づき作成しているものですので、万人に共通するものではないかもしれませんが、レビューで建設的なコミュニケーションがされると、非常に気持ち良いですので、ご参考いただければと思います。