【初心者向け】非エンジニアチームにおけるコードレビューの心構えと進め方

エンジニア文化に馴染みがないチームを立ち上げてコードレビューをトライする機会が増えてきたので、その思想と心構えについて書きます。

レビュー初心者や、エンジニア初心者が多いチームの方も参考になるかと思います。

コードは作る時間よりも保守する時間の方が圧倒的に長いです！
しかし、みんなで構ってあげられるのはリリース前が実情なのでコードレビューを実施して品質を確保しましょう。

はじめに

コードレビューはその性質上、コードの作成者にとってネガティブなことを伝える必要性が生じます。

この時、伝え方に気を付けないとチーム内がギスギスしてしまい、全体のパフォーマンスが下がってしまいます。

また、エンジニア文化に馴染みがない、薄い人が多く集まる開発チームで行う場合、
逆に言うべきことが言えず(わからず)なあなあになることもあります。
(通してはいけないコードが通ってしまう)

以下、基本的には対面やオンラインでの同期レビューを対象に書いていきますが、Pull Requestなどの非同期レビューにも通じるように書いていきます。

チーム内で展開するため、レビューするコード(ツール)のUiPath目線で一応書きますが、言語・ツールに関わらず通じることが多いと思います。

また、ドキュメントのレビューに対しても適用できます。

以下、コード＝成果物とざっくり読み替えてください。

この記事でわかること

• 非エンジニアチームでのレビューの心得がわかる
• 対面・オンラインMTGでのレビューのやり方がわかる

この記事の対象者

• 非エンジニア、または初心者が多い開発チーム
• レビュー初心者
• RPA(UiPath)開発チーム

レビューの目的

• チームの文化を作る
  • コードと書き方、考え方を共有できる機会になる
• チーム全体のスキルを上げる
  • 他人の書き方を学べる
  • 自分の書き方を教えられる
• コードの品質を向上させる
  • 他人に見てもらうことを前提に書くことで、汚いコードが放置されることを防ぐ
  • 他人が見ると意外とすぐ間違いや設計漏れに気づくことがある
  • 議事録を残すことで品質の記録として残すことができる
• 効率化
  • レビューにより綺麗なコードになる
    • 綺麗になると後から見た人(自分含め)の学習コストが下がる
    • 綺麗になると機能変更や修正がしやすくなる

用語

基本用語

• コード:ここではレビュー対象の作成物のことをひっくるめてコードと呼ぶ
  • ソースコード
  • RPAのロボット
  • ドキュメント
  • 設計図
• レビュー:コードを作成者以外の人が検証すること
• レビュアー:レビューする人(見る人、承認する人)
• レビュイー:レビューされる人(作った人)
• リファクタリング:ここではざっくりコードの中身を綺麗にすること。機能追加やバグ修正はリファクタリングではない

プログラムでよく出てくる原則

プログラミング・ITエンジニアの世界では原則・格言めいたものが多数あります。よく出てきて知っていた方がいいものを挙げます。

• DRY原則:本質的に同じものは1箇所にまとめると良いという考え
  • 例:処理するファイルパス以外は同じ処理をする場合、関数化して引数でファイルパスを受け取った方がいい
• KISS原則:コードを書く時はシンプルなのが一番良いという考え
  • 例:単純な足し算のロジックに複雑なループを組まない方がいい
• YAGNI原則:ヤグニと読む。必要になるまで最低限の実装しかしない方がいいという考え
  • 例:年齢で何かを判断するとき、300歳以上の場合の処理は考えない方がいい

レビュアー・レビュイーの心構え

• レビューする、されるのはコードのみ。レビューで人格攻撃はしてはいけません

最も大事なことなので以下の場合お互い読み上げましょう。

• レビュアー・レビュイーどちらかが初体験
• 新しいチームメンバーが入ってきてから初のレビュー
• 新年度など

オレ、オマエ、トモダチ。

• 人の好き嫌いは一切関係ありません
• コードの「正解」はありません。より良いコードがあり、それはいくつもあります
  • プロジェクトの状況、時間軸、様々な制約で最適解にはならないことも多々あります
• レビュアーとレビュイーに上下関係はありません
• 双方きつい言い回しをしないようにしましょう
  • 不必要に攻撃的な言い方はしない方が身のためです
• 指摘は具体的に行い、文書など残る形で残しましょう
  • 対面・オンラインMTGで行う場合、議事録を誰が書くかは決めておきましょう
  • 議事録を書くことも勉強になります

レビューの進め方

チームの立ち上げ時はその後の文化定着なども考えてなるべくみんなで集まりましょう(対面・オンラインMTG)
正式なレビュアーは1人とし、その他は後述のコメント種類に従って発言すれば良いと思います。

開催

レビュイー

• テスト実施
  • 仕様を満たしている動作をしているかを自分で確認しておきましょう。実行記録も残しておくと良いです
• できる限りのリファクタリングをしておきましょう
  • コード規約の確認
  • Linter
  • Formatter
• 会議案内を出しましょう
  • レビュアーの設定
  • 議事録担当の設定
  • 日時の設定
• 説明に必要なドキュメント・資料を用意しておきましょう
• レビューまでに資料をテンプレートに沿って作成して共有しておきましょう
  • テンプレートは議事録と一体化したものが望ましいです

レビュアー

• 共有された資料、コードに目を通しておきましょう

レビュー

1. いろいろ余裕ある場合はアイスブレイクをして和んでおきましょう
2. 何のレビューか、誰がレビュアーで誰が議事録担当かを最初に表明しましょう
3. 議事録担当はレビュー中にリアルタイムで議事録を作成していくのが望ましいです
4. 事前共有した資料に沿ってレビュイーが説明しましょう
   1. 背景の説明(参加者の理解度に応じる)
   2. 仕様の説明
   3. コードのふるまいについて説明
   4. 気になっていることなどを共有
5. 資料をもとにレビュアーがレビューを実施しましょう
   1. 背景・ドキュメント・仕様に認識の相違がないか確認
   2. コードに対してコメントの種類を表明しながらコメント(後述)
6. 他の参加者がいればレビュアーと同様に疑問やコメント
7. レビュイーがタスクを設定、レビュアーが同意する※レビュイーが経験浅い場合はタスクの設定から周りがサポートしてあげましょう
8. クローズ

レビュー後

1. 議事録担当者はなるべく当日中に議事録を参加者に展開し、間違いがないかを確認しましょう
2. タスクが設定された場合、担当者は処理しましょう

レビュアーの心得

• 指摘する場合は根拠を必ずつけましょう
• 良い箇所は必ず褒めましょう
• なるべくレビューを1回で終わらせましょう
• あらさがしは極力少なくしましょう(後述の[NITS]は1レビュー5回程度まで)
• レビュアーはコメントする場合に口頭、文書問わず必ず以下のどれにあたるかを表明しましょう
  • 上に行くほど重要度が高いです
  • ユーザー辞書に登録しておきましょう

コメント種類	意味/例
[GOOD👍]	いいね！ LGTMともいいます1 [GOOD👍]ここの処理がスマート！天才！
[MUST]	絶対直してください、直さないと承認できません 仕様を満たしていない、不具合が起きていたり、起きたら大変そう 強い言い方にならないように気を付けましょう [MUST]個別に保存すべきデータが上書きになっているので修正お願いします！
[WANT]	できればこうしてほしい、修正してほしい [WANT]エラーを全てexeptionでキャッチしていますがこれはビジネス例外なのでできれば別に分けてください
[IMO🍠]	in my opinion 個人的な意見 こうした方がいいかも。この方がレビュアーは好き。 [IMO🍠]きのこよりたけのこの方が美味しいです [IMO🍠]この書き方の方がリソース少なくて速そうです'hogehoge'
[ASK]	単純な疑問、純粋な質問 質問風批判ではありません [ASK]AとBの処理を分けているのはなぜですか？
[INFO]	情報、参考まで 前調べて同じようなことになりました。URL…
[NITS]	Nitspick あらさがし、ささいな指摘 タイプミスの指摘とかルール内での変数名、表記ゆれの指摘など 多用されるとだるいので1レビュー5回程度まで [NITS]UipathではなくUiPathが正しい表記です

レビュイーの心得

• コードと対象プロジェクトに1番詳しいのはレビュイーです
• レビューに持ち込む前に必ずLinter(コードの問題点を指摘してくれるツール)やFormatter(インデントなどを整形してくれるツール)を使用して可能な限り機械的に判断できる部分の指摘を潰しておいてください
  • UiPathの場合、UiPath Studioの「ワークフローアナライザー」機能を使用してください
• できる範囲で動作確認やテストを行い、結果を確認・記録しておいてください
• テンプレートを使用して事前にレビューまでに情報を共有しておいてください
• レビュー範囲は少なければ少ないほどお互いの負担が少ないです。いきなり全てのコードをレビューするのは大変です
• 良い機会なので疑問点やより良いアイデアをレビュアーや参加者に求めても良いです
• 普通と違うことをしたときはちゃんと説明できるように準備しておきましょう

議事録担当の心得

• 誰が発言したかを必ず記載してください
• レビューコメントは種類を必ず記載してください、書かないと重要度がわかりません
• TODO、つまりタスクは必ず記載してください
• 同時編集できるドキュメント形式だと嬉しいです
• あとは基本的に参考資料通りです

参考資料

おわりに

レビューが 地獄 になるか、良いコミュニケーションの場になるかはチームの管理側、レビュアー側にかかっていると思います。

経験上、1回険悪、もしくは事なかれ主義になってしまったチームを立て直すのはかなり大変なので立ち上げ時にかなり意識していく必要があるのかなと思います。

1. LGTM(Looks good to me) ITエンジニア界隈ではよく使われる言い回しですが、非エンジニアには全然馴染みがないのでもっと伝わりやすい[GOOD👍]を採用しました。褒め言葉だしね。この辺はチームのバックボーンで選べば良いと思います。 ↩