こんにちは、サーバーサイドエンジニアの山田です。
みなさんコードレビューしていますか。私の所属するチームでは全員でレビューを行なっています。(全員の承認を待つ必要はありません)
私は人のコードを見るのが好きなので、基本的にはレビュー依頼を受けた全てのプルリクエストに目を通すようにしています。
今回はコードレビューでやっていることや思っていることを書いていこうと思います。
コメントにラベルをつける
よくある方法ではありますが、私たちのチームでもレビューコメントにラベルをつけています。
各コメントに対してどういう対応を行えば良いか分かりやすくなるかなと思います。
主に使用しているのは以下になります。
- MUST
- WANT
- IMO
- ASK
MUST
修正必須のもの。基本的には修正しない限り承認されない。
- 仕様を満たしていない
- セキュリティの問題がある
- パフォーマンスが著しく悪い
WANT
修正必須ではないが、できれば修正しておきたいもの。
時間があれば修正する、チケットやコメントに残して後で対応できるようにする。
- 言語/ライブラリの一般的な使用方法から外れている
- 設計や命名がリポジトリの方針から外れている
- 近い将来負債になる可能性が高い
- パフォーマンス面に問題がある
IMO
個人的な意見や好み。将来的にどうしていきたいか等。
良いと感じたら修正する。
ASK
ただの質問。返答が必要。
良いと感じる部分をコメントする
レビューの場合、どうしても良くない部分を探す作業になってしまいます。
逆に良いと感じる部分にもコメントしていくのが良いかなと思っています。
褒められるのは嬉しい、どういうコードをかくのが良いのかの判断になる、と思うためです。
できるだけ早めにレビューする
レビュー依頼を受けたら遅くとも1営業日以内にはレビューするようにしています。
レビュー待ちのものが溜まっているのは良くないかなと思います。
自身の作業との調整が必要ではありますが、基本的にはレビューを優先する方がチーム全体のパフォーマンスはあがるのではないかなと思っています。
あまり理解していないものでもレビューに参加する
チームに入ったばかりの時や経験の浅い技術の場合は、中身を見てもよく分からないと思います。
ただ、そういうときこそレビューに参加する(コードを読む / 他の人のコメントを確認する)のが良いと思っています。
一時的に自分の作業が遅れることになりますが、それ以上にレビューに参加した経験げ後々活きてくると考えています。
事前にプロジェクトの概要を共有する
全員でレビューを行っているため、自分の参加していないプロジェクトの修正についてもレビューに参加します。
そういった場合はプルリクエストの内容だけを頼りにレビューを行うのは大変です。
そのため、私たちのチームでは事前にプロジェクトの概要を全員に共有する場を用意しています。
プルリクエストの単位を小さくする
プルリクエストの単位が大きいと内容を確認するのが大変です。
意味のある単位で小さい変更を心がけるのが良いかなと思っています。
プルリクエストが小さければレビュー完了までの待ちも減り、回転がよくなるのではないかなと思います。
ただ、プルリクエストの単位を小さくすると、最終的にどんなことがやりたいかの全体像は見えにくくなります。
(やりたいこと全てを含む大きいプルリクエストだからこそ気づける点もあるはず)
レビューする側は「事前にプロジェクトの概要を共有する」である程度全体像を理解しつつ、想像しながら確認する必要があるのかなと思います。
コメントをもらいやすいように事前に準備する
自分の中で100点のコードになってからレビューを依頼する、ということはあまりないのかなと思っています。
改善したい点がありつつも良い案が浮かばない時など、他の方の意見をもらいたい場合は、事前にプルリクエスト上やソースコードにコメントを残すのが良いと思います。
そうすれば、それを見た方が意見を言いやすくなるのではないかなと思います。
さいごに
個人的にはコードレビューは意見交換の場になるのが良いのかなと思っています。
ただ問題のある点をあげるだけではなく、今後どういうコードを作成していくのか、より良いコードにするにはどうするのがよいかを相談できると良いなと思っています。
ユニファでは一緒に働く仲間を募集しています。
ご興味を持っていただけましたらぜひ採用情報をチェックしてみてください！ jobs.unifa-e.com
