色々決めごとをするときは社内にissueなりwikiなり残すのですが、これは外に公開してもいいんじゃない？と思ったので公開します。 今回は「コードレビューの導入する」というやつです
なぜコードレビューをするのか
大きく二つの効果があると思っています
コードの品質を保つ
一人でコードを書いたときに、それが対応として適切であるかどうかを判断するのは結構難しいです。 基本的には機能を実装するとき、以下の三つについて確認する必要があると考えています。
- コードの書き方が適切であるか
- 解決方法が適切であるか
- 修正箇所以外の影響への対応は適切であるか
下に行くほど考えるべき領域が広くなっていきます。 領域が広くなるほどに一人で対応できる難易度は上がっていくため、漏れが発生し不具合が起きてしまいます。 特に３に関しては自分が予想だにしていないところに影響を与えるなどありがちです。 これを防ぐためにテストコードを書いたり、レビューにより他人の視点、知識から問題を発見していくことが重要になります。
コードの属人化を減らす
ゲームは特に専門性の高い部分が多く、この部分はこの人以外対応できないという問題が発生しがちです(属人化)
短期開発における属人化は速度が出せるので有効ですが、長期の開発においての属人化はボトルネックになりがちです。 これを回避するためには全員がある程度コードを把握しておく必要があります。 コードレビューは自分が把握していないコードを見る有用な時間です。 品質を保つためのレビューだけではなく、自身がコードの内容を把握するためにレビューしていく習慣をつけることで属人化に対処できると考えています。
どうやっていくのか
PRのマージは2人のapprovedを必須にする
- 2人のapprovedを確認して、自分でマージしてもらいます。(書いた人が取り込みたいタイミングを理解しているはずなので)
- 人数は暫定です。多いか少ないかはやってみて動的に変えていこうと思います。
- 2人approvedもらえたら、そこからいつマージするかはPR出した人にお任せします。状況に応じてもっとレビューを求めるのも問題ありません。
非スクリプトファイルのみの変更はレビュー不要
- エンジニアが目視で見ても判断つかないため
- レビュイーがUIテストなどが通ってるかは確認しましょう(いずれ自動化)
- 「レビューポイント」のところにレビュー不要な理由を書きましょう(シーンファイルのみのためetc..)
インゲーム・アウトゲーム関係なく2人をランダムにレビュアーにする(ランダムな仕組みは後から作る)
- 属人化を防ぐため
- リード的な人のレビューを必須にするとボトルネックになってしまうため
- この人に見てもらわないと不安！みたいなシチュエーションの場合はその都度その旨書いて指定する
レビューを優先する
- レビューが終わらないのでマージできない！という状況を防ぐために、レビューを優先！という共通認識を持ちたい
- 忙しすぎてレビューが本当に無理！ということになったらタスクの積み方から一旦見直す
レビュイー(PR出す側)に意識してほしいこと
PRの粒度を意識する
PRのコミット粒度を小さくすることを心がけていきましょう。 体感ですが、スクリプトファイルの変更が二桁突破するともう辛くなります！ コミット粒度を小さくするためには、エンジニアタスクの適切な分解が必要になってきます。
例えば「APIからとってきた情報を表示する新規の画面の追加」というタスクがあるとしたら、それは以下のタスクに分解できます。
- 画面に表示する情報を持つモデルを作成する
- 新規のAPIと通信をするメソッド実装
- 新規画面の追加(データは決め打ちのモック)
- 画面にモデルの情報を表示するように繋ぎこむ
これらをまとめてやってしまうと肥大化してレビューしにくいので、適切な分割を心がけましょう、
背景が伝わるように文章を書く
PRから背景が伝わるように文章を書いていきましょう。 なぜこのPRをやるのか、どのように解決するのか、特にどこをレビューしてほしいのかなのかがハッキリすればレビュアー側の負担を減らすことができます。JIRAチケットのURLを貼るのもいいでしょう。 前職では以下のようなテンプレートを採用していました。これに近いものを用意しようかと考えています。
https://gist.github.com/adarapata/40ec5f66e0c348a639a1aa6cb519aee6
レビュアーに意識してほしいこと
HRTの原則を守る
「レビューはBARで語らうように」という名言があります。レビューとは相手のやったことに対してコメントするので、ともすれば相手を傷つけてしまうような悪い空気になってしまいます。それが発生しないように謙虚(Humility) 尊敬(Respect) 信頼(Trust) の気持ちをもってコメントしましょう。
HRTの原則 ～ソフトウェア開発はバーでしっとり語り合うように ～
http://blog.livedoor.jp/lalha/archives/50496623.html
なぜを書く
「良い」「悪い」という基準は主観的な言葉なので、どういう観点から良い、悪いを判断したのかを書くように心がけましょう。
悪い例：「ここはBのようにしたほうが良いと思います」 いい例：「ここはBのようにするとネストが減り可読性が上がりそうです」
わからないところは質問していく
レビューは品質の保証だけでなくレビュアーが要件を理解する場でもあります。なので、こうしたほうがいいというコメントだけではなくて「このコードは何をしているんですか？」みたいな自身の理解を深めていくのも目的です。 なのでこの辺りがわからない、というのもどんどんコメントしましょう。
褒める
めっちゃええコードやんと思ったらそういうコメントもどんどんしてください。
