コードレビューで楽をしたい
この記事はRecruit Engineers Advent Calendar 202218日目の記事になります。
対象読者は主にコードレビューをする人になります
楽を目指すコードレビューの定義
一言にコードレビューと言っても形は様々で、それぞれに特徴がありますが、本記事で楽を目指すレビューは経験上多くのプロジェクトでその承認を持ってコードの受け入れをしている
pull request によるコードレビューとします。
レビューの目的
レビューの目的は、後で困らない状態を作ることです。
困る状態とは
- 期待した挙動になっていない
- 開発が間に合わない
- メンテナンスできない
などが挙げられます。
レビューの負荷を下げるには
開発をしていく中でレビューがボトルネックになることがあります。このような事態が発生すると、レビューの漏れが発生するなど質が落ちてしまい、解消すべき困りごとが溜まってしまいかねません。
こうならないための解決策を見ていきましょう
CI ツールを作成する
CI とは Continuous Integration の略で日本語では継続的インテグレーションツールと呼ばれるものです。手元で走らせている test や formatter, build のチェックなどをトリガーを指定して,コード push や pull request の度に自動実行させることができます。これによりレビュアでチェックすべき項目を減らすことができます。
フロントエンドの開発においては、Visual Regression Test を加えるとデザインの差異まで確認が取れるので意図しないデザインのズレが発生していないかなど影響範囲を確かめるのに非常に便利です。
書き方を制限させる
多くのプロダクトがフレームワークを使ったり独自のボイラープレートを作りそれをベースに開発をしていると思います。
はじめに各モジュールごとの書き方を決めきってしまい、ドキュメントさらにはそれぞれの generator の作成までしてしまえると実装者が手を加えた箇所が明確になり、確認すべき設計・コードの量を減らすことができます。
ペア・モブプロを適度に行う
※ここでのペア・モブプロ（以下モブプロ）は対面ないし・オンライン通話などで画面を共有し会話しながら実装を進めていくもの程度の意味合いで使っております。
モブプロの大きな特徴は、リアルタイムコミュニケーションである、コード以外の情報を知れることです。PJの立ち上げ期間や、実装上の詰まりが発生した場合スポット的に開催する、モブプロは特に大きな効力を発揮する場合が多いです。
誤った方向に進まないための軌道修正だけでなく、教える側はメンバーの技術レベルを知ることができたり、コードの書き方以外にもツールやその使い方を知らずに無駄に開発時間が嵩んでいるなどの問題を早い段階で解決できることもあります。
立ち上げ期のモブプロは複数回実施されることが多いですが、モブプロのみの開発が続くと集中力、吸収効率が下がり、ストレスも溜まりかねません。適度に個人作業を振り、困りごと・わからないことを自分で見つけてもらい、改めてモブプロを開催して困りごとの解消を行っていくと効果的です。
その他メンバ全員が触る共通モジュールを作成した際、メンバが持っているタスクの難易度を見て進め方に悩みそうな場合等にペア・モブプロを開催すると、後に待ち受けるコードレビューが楽になります。
テンプレートを設ける
プルリクエストテンプレートを設ける目的は、確認、やり取りの回数を減らすこと、後から見返した時に何をしたかが明確な状態を作ることです。
大規模開発だと多い時は1日20件のレビューが上がってくることもあり、それらをタイトルだけ見て仕様を探しにいったり、確認のやりとりをしたりするのは不要な時間がかかってしまうため、レビューするのに困らない情報をはじめから入れてもらうようにしています。
私がよく使うテンプレートは以下です
### やったこと
### やらなかったこと
### 変更理由
### 見て欲しいポイント
### 参考リンク
### 確認方法
具体的に埋めた例
やったこと
- フッターコンポーネントの作成
- コンポーネント利用サンプルの作成
やらなかったこと
- 全ての利用ページへのコンポーネント埋め込み
- 後続タスクで実装予定
- 各種リンクの url の設定
- 仕様が固まっていないため
変更理由
新規追加
見て欲しいポイント
- コンポーネントの切り方は適切か
- テストでのDOM要素取得方法
参考リンク
確認方法
npm run dev
open http://localhost:3000/home
フッターがデザイン通りに表示されること
各項目の詳細は以下になります。全項目必須とはしていませんが、基本的に
参考リンク・変更理由 のどちらかは必須で入れてもらっています。
### やったこと
文字通りやったことを書きます
### やらなかったこと
この pr 内では行わなかったことを書きます、後で忘れないようにソースコードの中にも TODO コメントをつけてもらうようにしています。
これにより、レビューのスコープはどこまで見れば良いのかが分かるので「この実装抜けているように見えますが大丈夫ですか?」などのやりとりをスキップすることができます
### 変更理由
新規追加・不具合解消 が主な内容です。
外部リンクとして貼れるようなまとまった資料のない修正を行った場合にはここで理由を書いてもらいます。
### 見て欲しいポイント
実装していて不安になった特に見て欲しいポイントを書いてもらいます
### 参考リンク
実装の参考にしたリンクがあった場合必須で入れてもらっています。
### 確認方法
不具合解消などの場合はその解消ができる手順を記載してもらいます。
レビュアを増やす
コードレビューはある程度技術力がある人に集中しがちで、負担がかかりすぎるとレビューの質もスピードも落ちてしまいます。特にレビュアが上述のような整備を行うリーダー人格の場合目指すべき姿は自分がいなくなっても大丈夫な状態を作ることになるので少しずつメンバにレビュータスクを振りできる業務範囲を広げていきましょう。
まだ実装に不慣れでレビューに不安が残るという場合には仕様と技術のレビューを分けて行う方法もあります。仕様レビュアは主に仕様書どおりに動いているか、デザインと実装にズレがないかなどの部分を確認し、技術レビュアはバグを生みかねない実装やおかしな設計がないかを確認します。
終わりに
コードレビューはプログラムを書く仕事をしていたら切っても切れない存在です。誰しもが最初はレビューされる側で、徐々にする側を担当していくことになると思います。
できるだけ事前対策をしつつ、タイミング・メンバごとにどのような向き合い方が良いかを見極めながら進めることができれば、コードレビューで見るべき箇所が減り、一つ一つのレビューが楽になり、結果としてレビューの質を上げることもできるのではないでしょうか。
Discussion