ニフティでもコードレビュー会をやっています

こんにちは。WEBサービス開発グループ 砂川です。@nifty不動産の開発チームに所属しています。
本日は我らが不動産チームで行っているコードレビュー会についてお話したいと思います!

テーマは以下の3点です。

  • やり方や観点
  • 効果
  • 気をつけたいこと

どんな感じでやっているのか?

毎週金曜日に約30分間開催しています。

やり方としては、

  • 1週間前に、コードレビューされる人が立候補もしくは指名を受ける
    • その週に作成されたプルリクエストが選ばれることが多いです
  • 開催日までにコードをレビューしてコメントを付けあう(レビュアーは全員、もしくはテーマによって選抜
  • Bitbucketの機能を使ってコード内にコメントを書き込みます。観点としては、
    • 要件に対して実装に漏れが無いか
    • ロジックは正しいか
      • 処理ロジックの設計
      • 命名規則の妥当性
      • クラス・関数・変数
    • 例外処理
    • あらかじめよくある簡単なエラーを確認
      • ifは全て通るか?
      • Off-by-Oneエラーなどは無いか?
      • ・・・などを気をつける
  • 当日のコードレビュー会で、コメントを紹介しつつ、みんなで議論します。

準備が間に合わない場合などは当日ライブコードレビューを行うこともありますが、それはそれで面白いのでお試し下さい。

こういった感じで、割とゆる~い感じで毎週開催し続け、半年以上続けることができました。

効果は?やってよかった?

やってよかったと思います。
2年目エンジニアである私の主観的な感想を以下に記します。

1.お互いの業務内容の理解

@nifty不動産は非常に大きなサービスのため、「この人はWEBサイト」「この人はiOS」「この人はDB」という状態になりがちで、お互いのやっている作業やコーディングに対して深く立ち入ることは多くありません。
しかし、コードレビュー会を開催することで「こないだ言ってた作業ってこういうことか!」という理解の手助けになります。
属人化とは逆の流れを生み出せているなあと個人的には思います。

2.コミットの粒度とメッセージに気をつけるようになる→切り戻しもしやすくなる

めんどくさがって、「違う画面の改修だけど一気にコミットしちゃおー」ということをしなくなりました。
レビューしてくれる人に申し訳ないな、という気持ちになるからです・・・・・・。
一つの機能を作ったら、その都度コミットして、どういった改修を行ったかを書くというクセが付いてきて、結果的にファイルの切り戻しがしやすくなったり、この機能は残しつつあの機能は無くす、ということがしやすくなったと思います。

3.「初めて見る人が触っても問題ないか」という意識が強くなる

わかりにくい変数名メソッド名を付けるとか、インデントが変とかはありがちなのですが、人によって作業内容が大きく違う/入れ替わりも多いというチームの特性もあり、処理がすぐに理解できないコードは指摘を受けます。
「このコードを初めて見る人が理解出来るか」という意識でコードを書いたり、コメントを残すようになります。

 

ここでは紹介しきれませんでしたが、他にも小さな効果がありますし、意外とそんなに時間を使わないので、迷っていれば是非一度やってみることをオススメします。

 

< コードレビュー、最高!