LoginSignup
12
2

More than 1 year has passed since last update.

僕が考える最低限のコードレビューのマナー

Posted at

コードレビューをする側に人間になって思ったこと。
最低限のマナーがあるんじゃないかって思い始めた。マナー講師始動...!!!!!!

「相手に理解がしてくれるだろう」という前提で依頼するというのは、コミュニケーションとしても破綻してると思うので、相手に理解してもらう為に自分がまずできる出来る不安材料を取り除くのが大切だと思います。

自分が考えるコードレビューの時に必要な5箇条。

  1. プルリクテンプレートを用意しよう
  2. レビュー修正依頼の確認コメントはコミットIDで返答しよう
  3. 確認依頼のコメントを残そう
  4. 絵文字多用しよう
  5. 双方お礼を忘れずに

1. コードレビューにはプルリクエストテンプレートを用意しよう

PullRequest Template(プルリクエストテンプレート)はレビューの際に必須。
これがないとプロジェクトが破滅してるんじゃといった感じ。

なぜ、プルリクエストのテンプレートが必要なのか

  • コメントが書く人によって書くフォーマットがバラバラになり、毎回統一性のない書き方になる
  • 何のタスクなのか、プルリクから読み取れない
    • 本当に必要なタスクなのか
    • 誰が立てたタスクなのか
  • そもそも何を書けばいいのか不明といった疑問の解消

コードレビューをする上でも、何を見て欲しいのかコードからだけでは読み取れない。
レビューだけではなく、どういったコード修正をやったかコメントを残す意味でも重要だと思います。

弊社ではプロジェクトにもよりますが、テンプレートを以下のように設定しています。

## 概要


### Ticket/Issuesリンク
<!-- Ticket/Issuesリンクは必須です -->

- 

### なぜこのタスクを行うのか
<!-- 上記リンクがあれば不要 -->

- 

### やったこと

- 

### やらないこと

-

### できるようになること(ユーザ目線)

- 

### できなくなること(ユーザ目線)

- 

### その他
<!-- レビューアーに確認してもらいたいこと -->

- 

テンプレートの雛形は、他の会社の参考の事例を元手に作成しています。
完全に自分の会社・プロジェクトに合ったものは無いはずなので、部分部分で採用して使っていく感じがいいですね。個人的に以下のことが分かればいいかなと思います。(他はタスクや会社のルール次第)

プルリクテンプレートの必須項目

  • Ticket/Issuesリンク
    • GitHubのIssuesやJIRA, Backlogといったタスク概要のリンクを貼る
      • 最低限これは貼っておこう!
  • なぜこのタスクを行うのか
    • レビューアーがパッと見て何の目的でコードを書いたのかを簡潔に記述
      • Issues/Ticketのリンクなどあればいらない気もしますが自分は書く派
  • やったこと
    • このプルリク自体では何を対応したのか記述
      • e.g.1)ユーザーの登録機能を作成しました
  • やらないこと
    • このプルリクに含まないことを記述
      • e.g.1)このプルリクではテストコードは実装しないです
      • e.g.2)このプルリクでは~ファイルはリファクタリング対象外とします
  • レビューアーに確認してもらいたいこと
    • 何を見て欲しいのか
      • e.g.1)仕様にあった実装をしているのか
      • e.g.2)処理が効率的かを見て欲しいのか

その他、リリースチェックに関することとかも入れているところもありました。

  • エラーが出ていないこと
  • 画面崩れが起きていないこと
  • テストが成功していること

雛形を作成しておけば、ある程度入力を強制することができるので、何を書けばいいのか分からないって事もなくなると思います。

プルリクテンプレートの雛形について

以下がかなり参考になると思います。

2. レビュー後修正の確認依頼のコメントはコミットIDで返答しよう

修正依頼のコメントに対しては、その指摘分だけを修正したコミットIDを追記するやり方です。
※GitHubではコミットIDをコピペすると自動でリンクになります

レビュー後修正OKパターン.png

「え...そんな細かく?」と思った方いるかもしれません。
実際コミットIDがない以下のように返す方も多いです。

レビュー後修正NGパターン.png

ただ、実際レビューする側が見やすいのは明らか前者ではないでしょうか。
後者は「対象のファイルを見に行って、再度指摘箇所を探して...」とレビューする側に地味な作業負担を強いています。相手が見てくれるでしょといったスタンスでいますね。

逆に前者は修正文のリンクだけ置いてるので、そこさえ見れば指摘の修正がされているのかすぐ判断できます。指摘以外の修正が紛れ込んでいる場合でも気付く事が出来ます。

レビューする側は見終わったら必ず、「Resolve conversion」ボタンを押し確認済みということを明確化しましょう。
GitHub_Resolve_Conversion.png

3. 確認依頼のコメントを必ず残そう

レビューで指摘され、コードを修正後、再度レビューを相手に依頼する際にどのように依頼していますか?

  1. 無言でレビュー依頼のリクエストをGitHub上で行う
  2. チャット上で「確認依頼」を出す
  3. チャット上で「確認依頼」を出した上で、GitHub上でも「確認依頼」コメントを残す

1 ❌ 論外すぎてアウト 2 🔺 チャットをすぐ見て、反応してくれる人なら確実だけど...   チャットもグループチャットとかだと流れる可能性が高いので、相手によっては要注意。   後日見ようとして、相手が「あれ?これ依頼出されてるけどコメントもないし終わってるんだっけ?」となる可能性も高いです 3 ⭕️ 確実です   チャットで相手に通知を送っておいて、GitHub上でも以下のように残しておけば、後日レビューする側が「もう見てもいいんだな」と相手に確認を取らず、すぐレビューできます   e.g) 「上記指摘全て修正済みのため、レビューをお願いします」

相手がすぐ見て反応してくれるとは限らないし「これもう見てもいいんだっけ?」「やり直してる最中です」とかの不要なやり取りを無くす為にも、"終わっている"という履歴を確実に残した方が無難です。

Kanban方式のプロジェクトとかであれば、レビューといった項目に移動しておくなどでも対策できると思います。

4. 絵文字を多用しよう

4はぶっちゃけマナーでもなんでもないですが。「なんじゃそれ」と思った方多いのでは。

理由としては、コロナ禍になってリモート等も進み、よりチャット文化になった昨今では、文章だけのやり取りは結構冷たさを感じます。顔を合わせて会ったことがある方とかだと、特に感じる事は少ないですが、顔とかよくわからない方だと余計に感じるのではないでしょうか。

文章一つでも相手へ伝わる印象は異なります。

  1. ありがとうございます
  2. ありがとうございます!
  3. ありがとうございます!👍

絵文字を使って感情を表現することは、相手の顔が見えていない、文章だけでは表現しきれない部分をサポートしてもらうといった意味合いで、個人的に結構重要だと思っています。

  • 修正してくれたら 感謝の気持ちで「👍」
  • めっちゃ綺麗なコード書いてくれて、最高の意味で「⭐️」

堅苦しい文面ばかりだと、人間関係も懲りがち。
少し砕けて、相手との距離感縮めてみませんか?

僕は以下の絵文字多用しています(ほぼGitHub上でデフォルトであるやつ)

  • 👍 自分は8割これ
  • ⭐️
  • ❤️
  • 😁

多用し過ぎると、おぢさん構文になります❗️😅💦 適度大事カナ❗️❓

5. 双方お礼を忘れずに

やっぱりお礼は大事。
お互いに時間を削って行っているので、お礼を言って気持ちよく終わりましょう。

  • レビューする側
    • 修正ありがとうございます!
  • レビューされる側
    • レビューありがとうございます1

無言でタスクをクローズすんな。

最後に

偉そうに書きましたが、実は全てこれ自分が新人の頃に、上司に指摘された事全部でした。
新人の時にあれこれ言われた時は「こまけー!!!」と思っていましたが、相手の立場に立った時にこれくらいやっておけば双方気持ちよく仕事できるな、と数年働いて実感しました。

逆に相手に「こまけー!!!」と言われたら相手に合わしちゃっていいと思いますが、最初から相手に丸投げでポーンは辞めた方がいいと思います。知らんけど。

相手の事思いやって仕事しよう。

12
2
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
12
2