本記事は
【プルリクウィーク】
3日目の記事です。
💻
2日目
▶▶ 本記事 ▶▶
4日目
📚
はじめに
こんにちは。髙橋です。
プルリクウィークというイベントに執筆依頼を頂いたので、私が普段業務でプルリクエストをレビューする/レビューされるときに意識していることを書いてみようと思います。
あくまで私が意識していることという意味合いであり、全員こうすべき！と押し付ける意図はありません！
チーム構成と使用ツール
まず前提の認識を揃えるために、私が普段どういった環境で業務をしているか説明します。
- チーム人数：7名
- チーム構成：PM×1、PL×1(私)、開発メンバー×5
- 課題管理ツール：JIRA
- ソースコード管理：GitLab
私は業務で7名ほどの少人数チームに所属しており、PLポジションを任せてもらっています。
PLの私のざっくりとした役割は、まず顧客との打ち合わせで要望・不具合をヒアリングしてJIRAチケット化します。次にチームメンバーにJIRAチケットをアサインし、チケットに紐づいたプルリクを作成してもらいます。それをPLがレビューします。
※GitLabではプルリクではなくマージリクエストと呼称されますが、GitHubのプルリクとほぼ一緒です。
レビューに入る前に考えるべきプラットフォームエンジニアリング
突然ですが、プラットフォームエンジニアリングという言葉をご存知でしょうか。
プラットフォームエンジニアリングとは、開発者が効率的にアプリケーションを構築、デプロイ、管理できるようにするためのインフラストラクチャやツールを設計、構築、維持することを指します。これには、自動化、クラウドサービスの管理、CI/CDパイプラインの設定、モニタリングとロギングの実装などが含まれます。
とChatGPTが教えてくれました。
https://chatgpt.com/share/e3418af3-0be7-4e6f-92da-0f6b0353bf12
プラットフォームエンジニアリングに一義的な定義はないのですが、私の解釈では「プラットフォーム＝Git、CI/CD、監視ツール、ロギング等」と捉えています。このプラットフォーム達をエンジニアリングします。
本記事の文脈で言うと、「プルリクレビューの前にGit自体の設定、CI/CDの構築をちゃんとしよう」ということです。
- ツールでレビューできるところは人間がレビューせずCIで自動化する
GitHub、GitLab等大抵のGitツールにはCI/CD機能が備わっていると思います。
CI/CDに自動テストや静的解析を実施するよう設定し、「テストが落ちてます」「文法的に警告が出てます」というやりとりをプルリクからなくします。
テストコードが一部落ちている、些細な警告が出ているといった指摘に脳のリソースを割くのは非効率的です。これらはツールで確認できることなので、人間が担当せずツールに任せます。
- プルリクの作成・更新を通知する仕組みを整備する
プルリクレビューは得てして溜まりがちです。
対策の一つとしてプルリクが作成・更新されたら何かしらの通知が飛ぶよう設定しておくことが重要です。
一番手軽なのはメール通知設定をすること（この機能はデフォルトでほぼ必ずあると思います）。メールじゃ気付けない！ということであればSlack通知をさせたり、JIRA等の課題管理ツール側に通知したりと、やり方は色々あると思います。
プラットフォームエンジニアリングができたら、いよいよプルリクを作ってレビュー開始です。
レビュアー側が意識したいこと
- 実装者の思考を想像する
レビューの中で「なんでこんな書き方してるんだ？」と感じることがあると思います。
単純に実装者がミスをしているだけのケースもありますが、なにかのっぴきならない事情でこうした書き方をせざるを得なかった場合もあります。
（例：既存仕様との兼ね合い、外接システムの仕様都合など）
ソースコードだけをみて指摘するのではなく、実装者が何を思ってそう書いたのかを考察・想像することで別の視点が見えてくると思います。
- 攻撃的な印象を与えない
「〇〇は誤りですので✗✗に修正お願いします」
「考慮漏れがありますので、ロジックを見直してください。」
レビューはどうしても実装者のミスを指摘することが発生しうるため、字面だけ見るとレビュアーがレビューイを詰めるような構図になってしまいがちです。（レビュアーとしては攻撃してるつもりはまったくないにも関わらず）
レビューイが萎縮してしまわないように、良かったところも合わせて褒めるよう私は意識しています。
具体的には、レビューコメントの文頭に[GOOD]とラベルをつけて
「[GOOD]〇〇のロジックがすっきり実装できていて素晴らしいと思います！」
のようにコメントを入れます。
最初は思いつきでやってみたのですが、のちの振り返りでメンバーから「[GOOD]コメントが嬉しかったので自分がレビュアーのときも実践します！」と言ってもらえたので、割と効果があったんじゃないかなと思ってます。
レビューイ側が意識したいこと
- レビュアーは全知全能の神ではない
レビュアーも人間です。すべてのミスを拾ってくれるわけではないですし、誤った内容のコメントを入れることもあります。
レビュー通ったから大丈夫！と過信せず、自分の書いたコードは自分が責任を持ってクオリティーを担保すべきです。
- レビューは指摘の場ではなく情報共有の場
レビュアーの章で「攻撃的な印象を与えない」と書いた話と関連しますが、ソースコードレビューは指摘の場ではなく情報共有の場だと私は考えています。
自分の実装がアプリケーションにどういった機能を追加し、追加の過程でどういった議論がなされたのかを他メンバーに周知することができる点もプルリクレビューの一つの側面です。
- 巨大なプルリクを作らない
1つのプルリクで何十ファイル・何千行も差分を生むのは避けたいところです。レビュアーの負荷が大変なことになります。
プルリクが巨大になるということは実装者の担当してる課題チケットのスコープが大きすぎるということなので、チケットを分割して対応を小分けにするよう私は意識しています。
まとめ
- レビューの前にプラットフォームエンジニアリングをしよう
- レビューイにはのっぴきならない事情があるかもしれないし、レビュアーは神ではない
- レビューは指摘攻撃する場ではなく、情報共有の場
