プルリクエストのレビュー時に意識していること

この記事は、自分が所属している GAOGAO の社内ナレッジ共有ツールに自分がポストしたものをそのままコピーしたものです。個人のアウトプットとしても残しておきかったので、Qiitaに投稿しています。
内容は GAOGAO の総意ではなく、あくまで私個人の見解です。

P-Rレビュー

意識していること

• レビューすることによって、レビュアー・レビュイーともに成長できること
• レビューが再利用可能であること
• レビューでレビュイーの自尊心を傷つけないこと

実例

レビュー対象コード

ユーザーの詳細画面のビューファイル(ここでは Laravel の Blade ファイルであるとする)。
$user でユーザーのインスタンスにアクセスできる。

<div>
  <h1>ユーザー詳細</h1>
  <ul>
    <h2>ID</h2>
    <li>{{ $user->id }}</li>
    <h2>ユーザー名</h2>
    <li>{{ $user->name }}</li>
    <h2>Email</h2>
    <li>{{ $user->email }}</li>
  </ul>
</div>

レビュー箇所

このコードでは、 ul タグ の子要素として h2 タグが入っている。
ul の子要素として入れられるタグの中に h2 は入っていないため、 HTML の仕様に反している。

レビュー例

ベースとなるレビュー(超初心者・ググる癖がまだない人向け)

この Blade ファイルの HTML の組み方だと、 ul タグの直下に h2 タグが入ってて、 HTML の仕様に沿ってないかもです！😱
ul タグの直下には li タグ以外入れられないので、 h2 タグは li タグの内側に入れてもらえればと...！🙇‍♂️

<div>
  <h1>ユーザー詳細</h1>
  <ul>
    <li>
      <h2>ID</h2>
      <p>{{ $user->id }}</p>
    </li>
    <li>
      <h2>ユーザー名</h2>
      <p>{{ $user->name }}</p>
    </li>
    <li>
      <h2>Email</h2>
      <p>{{ $user->email }}</p>
    </li>
  </ul>
</div>

「html ul 仕様」でググると出てくる Mozilla のページを見ると、「許可された内容」という項目でこの辺がよくまとまっています！🙆‍♂️
https://developer.mozilla.org/ja/docs/Web/HTML/Element/ul#attributes

ポイント

• 字面の圧力が強くならないようにする
  • レビューは基本的にテキストによるコミュニケーションであり、お互いの顔色をみてニュアンスを判断することができない。レビューでは伝えたいことだけをそのまま文字にすると得てして字面の圧力がとても高くなりがちなので、「怒っているわけではない」「一緒に良くしていきたいだけである」というニュアンスをできるだけテキストに落とし込む。自分がよく使っている例は以下の3つ。
    • 顔文字、感嘆符、三点リーダ使う
      • 「内側に入れてもらえればと。」 -> 「内側に入れてもらえればと...！🙇‍♂️」
    • 命令形ではなく依頼形の表現を使う
      • 「内側に入れてください！」 -> 「内側に入れてもらえればと！」
    • 断定表現を避ける
      • 「仕様に沿ってません！」 -> 「仕様に沿っていないかもです！」
• 「何がいけなくて、どう直すべきか」を明確に伝える
  • 直し方がよく分からず再レビュー、とならないよう、修正後のコードを記載する(場合によってはレビュアーが修正して commit してもよいかも)
• ググりワードを伝える
  • 普段使っているブラウザでググるとググり方を知っている人に最適化された結果が返ってくるので、シークレットモードで検索を掛けて結果を確かめるとよりよい
• 個人ブログやQiitaなどではなく、できるだけ公式ページ(もしくはそれに準ずるページ)を第一に伝える
  • ただURLを貼るだけでなく、該当する公式ページの読み方も合わせて伝える
  • 公式ページに載っていない内容、及びより実例に寄せた内容の記事であれば公式以外も共有してOK。できるだけ公式ページと一緒に伝える
• (発展) リンクを貼ったページを読んでもらえたかどうかの確認が出来るようにする
  • 今回の例だと、「ul タグは li 以外入れられない」とレビューしているが実際は script タグなども入れられる。レビュイーが「ページ見ましたけど、 script タグとかも入れられるみたいでしたよ」と言ってきたらしめたもの。

レビュイーのレベルに合わせたレビュー

レビュイーのレベルが上がるのに合わせて、順次上記のレビュー内容から引き算していく

• レビュー箇所の修正に対しての、レビュイーの自由度を上げる
  • 修正コード例を示すとその通り直さなければいけないような気がしてしまうが、実際は li の中の構造はもっと適切なものがあるかもしれないし、そもそもリストになっている部分は table タグで表現するほうが適切かもしれない。レビュイーのレベルが十分高いのであれば、修正の判断はレビュイーに任せる