エンジニアをしていたら、コードレビュー・設計レビューはほぼ毎日の業務で行う。
毎日やらなくてもレビューをする機会は必ずある。
そのレビューについて自分が意識してること書いておく。
前提条件
レビューとは、その実装や設計に対して上司・同僚への同意です。
レビューは、する人される人の価値観やスキルレベルで変わってくる。
リファクタした方がいいポイントは人によって異なったり、レビュイーとレビュワーで大事にしてることが違うとマインドの違いで本来の目的と違うところに向かってしまう。
そのためプロジェクトにおいてのマインドを揃えておくことはとても重要です。
また、揃えたマインドの定義は文書化することまで重要なポイントです。
マインドを揃えるとは、どこをみるのか。品質のレベル、コードの可読性など様々な視点を言語化、文書化しておくことが必要です。
なぜ?文書化が必要か。やはりプロジェクトはサービス終了するまでは半永続的に続いていくが、人は入れ替わるからです。
ドキュメントも大事だけど、このマインドの定義はReadmeにおくと途中から入ってきたエンジニアにも伝わりやすいです。
どこにおくかはその時々やプロジェクトによって変わるけど、とにかくHow to useくらい見える場所に置いておくことがポイントだと感じてます。
基本的なマインド
技術的なマウントを取らない
他者自身への攻撃はしない
レビュイーは、レビューしやすい粒度でPRにする(必要に応じて別課題にするなど。)
レビューするときは、常になぜ?を持つこと
レビュイーは、実装した部分のコードの行について理由を書く(複雑な場合)
MUSTやFYIやIMOは、冒頭に書く
良いコードは褒めたり 絵文字を使って表現する
常にリファクタできるかの視点を持つこと
目的の機能に対して拡張性を考慮できているか(ドメインに対してのアプローチは正しいかどうか)
レビュイー・レビュワー同士で耳を傾ける
技術的なマウントを取らない
技術的なマウントを取らないことです。この技術微妙だからこうしたほうがいいだったり、実装的に古い書き方だったりとか。が挙げられます。
たしかに時には、非推奨の書き方になってる。そもそも言語仕様として正しくない部分は指摘する箇所ではあるものの、なぜ?こうしたのかこれを選定したのかをきちんとレビュイーの意見を聞きつつ進めることを意識しましょう。
他者自身への攻撃はしない
能力的に微妙な部分を相手自身を攻撃するようなことはしない。感情でレビューしないことが挙げられます。
当たり前のことですが、チームとして働く以上相手の感情もコントロールすることは必要です。
感情でドンパチしてしまい、チームとしての機能が失われ、この人とはやりたくないなという感情が芽生えてしまい本来の目的の良くしていく活動に支障がおきます。
レビュイーは、レビューしやすい粒度でPRにする(必要に応じて別課題にするなど。)
相手にみてもらうということがレビューです。
その機能の File Change の量が多ければ、見る範囲も広がり結果レビュワーの時間が多くなってしまいます。
機能が大きいものは分割してPRを行うことを意識しましょう。
小さいレビューをすることで、タスク自体の進行もスムーズに行えます。
レビューするときは、常になぜ?を持つこと
目くじら立てて なぜ を探す必要はありません。
しかし、ん?と思うことは常に持つようにしましょう。
目的の機能の実装でも、実装自体をレビュワーも理解していなければ話がスムーズに行えません。
常に仮説を立てて、この実装はこういうことでこうしているのかい?
レビュー内でやりとりすることが、お互いの実装の解像度をあげていき言語化していきましょう。
レビュイーは、実装した部分のコードの行について理由を書く(複雑な場合)
PRを説明欄に要件などはプロジェクトのルールに応じて記載します。
しかし、複雑な実装部分ではレビュワーが読む必要があり、レビュワーのなぜの部分と多くなるため、実装のコードの行にはコメントや理由などを記載することでレビュワーへの見るポイントを軽減することができます。
ここの型エラーの解決方法がわかりません や ここの実装見通しが悪いのでなにか方法ありますか? といった。レビュイーからPRのコメントに対しての質問などもチームレビューの活性化に繋がります。
MUSTやFYIやIMOは、冒頭に書く
レビューは英語や日本語など、ケースは様々です。
最初の冒頭に MUST(必須)といった部分を書くことで読み手が意識して読みます。
これがあるとないとでは、この文章の結論どうしたいのか分からず余計なコミュニケーションが発生します。
ラベルがあると、このコメントがなんであるかが人目でわかるため意識的にかきましょう。
良いコードは褒めたり 絵文字を使って表現する
レビューは赤ペンみたいなイメージを取りがちで採点するものではありません。
学校では良い解答について、よくできました!みたいなハンコを押されるあれと一緒で、褒められる部分はどんどん褒めて伸ばしましょう。
絵文字やGoodだったりの感情は、世界人間的な標準的な文化だと思っています。
常にリファクタできるかの視点を持つこと
コードの量が多くなると、可読性が下がったり処理が多いと理解に時間がかり結果テストしづらくなります。
常にリファクタリングできる部分は、指摘・もしくはIMO的な表現で書くことをおすすめします。
プロジェクトのマインドになるため、スピードとの兼ね合いでケースバイケースで書くようにしましょう。
リファクタリングが目的になるのではなく、将来のコードの保守性と納期とのトレードオフで考えましょう。
目的の機能に対して拡張性を考慮できているか(ドメインに対してのアプローチは正しいかどうか)
目的の機能が将来の拡張しやすくなっているかがポイントになります。
設計レビュー段階で分かる部分は、将来こうありたい・こういう機能も追加したいというのは出てくるはずです。
必ずしも、PRのタイミングで拡張性が高いかどうかは判断し辛い部分はあります。
例えば、一つの関数があってその関数が全体に行き渡る場合は固定値といった部分でネックにならないかなどがポイントで見ています。
そこでしか使わない・将来拡張することは限りなくゼロであれば、考慮する必要性はありませんが常に拡張性は意識しておくと良いです。
時間があれば。。常に拡張することを意識していきたいですね。
レビュイー・レビュワー同士で耳を傾ける
チーム開発ですので、ときには 感情論や実装の好みが入るケースはあります。
まずは対話が大事なことですので、常に耳を傾け相手の意見や自分の意見を言いやすい環境をマインドとして定義しておくと良いでしょう。
まとめ
スキルレベルや経験値で見るポイントが変わるレビュー。
その経験をしていない実装者であれば、きちんとレビュワーの意見や経験値について深掘りできるような 心の余裕 を持ちましょう。
たくさんの失敗や経験・相手の経験などやベストプラクティスを常に追っていけるようなレビュー文化を作れるとよいと思っています。