0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

円滑なコードレビューを実現するために意識したいこと11選

Posted at

これはなに?

チーム開発におけるPRレビュー(=コードレビュー)を効率的かつ健全に進めるために意識したい点をまとめる。

  • レビュイー(実装者 && レビュー依頼する人)
  • レビュワー(レビューする人)
  • チーム

この3者の観点で工夫できることについて、実装からPRマージまでの一連の流れに沿って語っていく。

実装〜PR作成前フェーズ

1. commitは綺麗にしておく

commitログを頼りにレビューする人がいることや、未来に過去commitを参考したり特定commitをrevertしたりするケースもあるため、commitは綺麗にしておくのが良い。

「1つのcommitに含める変更量を小さくする」「意味のある単位でcommitをまとめる」「commitメッセージを真面目に書く」などなど。
この辺は、チームとしてルールを決めたり生成AI活用したりすることで、少ない労力で対応できる。

2. PR作成前にセルフレビューする

実装が一通り完了したら、実装者本人が変更内容を確認する。
客観的に自分の作業内容を見返すと、何かしらのミスは見つかるのでこのタイミングで潰しておく。
ミスが見つからない場合、セルフレビュー方法が悪い。(過激派発言)

「目的に沿った変更を加えていること」「関係ない箇所について変更を加えていないこと」「Linterやformatterが適用されていること」などなど。
セルフレビュー観点は、チームの開発規約や任されたタスクに応じてよしなに。
生成AIのおかげで、このセルフレビューの質はかなり向上し、実装者の負担も軽減されたのは嬉しいこと。

PR作成フェーズ

3. 差分行数が多すぎる場合はPRを分割する

レビュワー目線、PRが大きくなるにつれてレビューに必要な時間が増える。
人間の認知力には限界があるため、一度のレビューに例えば1時間以上費やすと、適切なレビューが困難になる問題がある。

加えて、レビュイー目線でも、レビュー依頼してからマージされるまでの時間が伸びてしまうことで不都合が生じる。
分かりやすいものだと「自身のPRがオープンしている間に他PRがマージされてしまうことで、修正対象にコンフリクトが発生し、その解消に追加の労力を割くことになる」といったケース。

というわけで、チームとしてPR1つあたりの差分行数の上限値を設定しておくのが吉。
具体的な行数は諸説あるが、500行など?
下記記事などが参考になるかも。

4. PRテンプレートに沿って最低限の項目を埋める

レビュイーは初手でPR概要欄を見るため、PR内容を理解するために必要な情報群を簡潔にまとめたい。

チーム開発しているならば、下記3項目は最低限PRに含めたい。

  • 何の対応をしたのか?
    • 要約と詳細
    • 意図や背景もあると良い
  • 動作確認
    • どのような観点と手法で動作確認をしたのか
    • 結果は?
  • レビュー観点
    • 細部のロジックまで見て欲しいのか、大まかに見れば十分なのか
    • 特に重視してレビューして欲しい点があるか

5. PR差分画面で補足コメントつける

レビュワーの属性に応じて補足情報をつけてあげると親切。
レビュワーの負担を軽減する目的や、せっかくのレビュー機会を有効活用しようという意図。

以下、一例。

  • プロジェクト参画したばかりの人がレビュワーなので、情報共有も兼ねてコメントつける
  • 忙しいマネージャーがレビュワーなので、最低限見て欲しい箇所に目印としてコメントつける
  • (プロジェクト歴長い人がレビュワーなので、特に補足コメントつけない、というのも大いにあり)

PRレビュー依頼フェーズ

6. レビュー期限を伝える

レビュワーも別タスクを抱えているため、依頼されたPRのレビュー優先度を考える必要がある。
その判断材料として期限は伝えるべき。

希望と必達の期限を伝えると良い。
また、マージ期限を伝えるでも良いし、初回レビュー期限を伝えるでも良い。

7. レビュワーを明示する

メンションなしでレビューお願いします、と投げたPRは基本的に誰も見ない。

これはレビュイー判断でレビュワー指定するのが難しい状況もよくある。
チームとして方針を決めておくのが良い。
ランダムbotでレビュワーを自動選出したり、持ち回りで実施したり……

ちなみに、レビュー判断でレビュワー指定する際は、レビュワーに指定した意図を伝えると親切。

PRレビューフェーズ

8. リスペクトを持ってレビューする

テキストコミュニケーションのため、表現に注意。

基本的に、対応してくれたことに感謝し、リスペクトを持ってレビューすれば問題なし。

9. 意図のわかるコメントをする

質問なのか、修正依頼なのか、ただの個人的な感想なのか、それが伝わる内容にする。

mustやwant、FYIなどのタグ(?)を活用するのも有効。

修正〜再レビュー

10. 質問に対して修正で返さない!!

レビュワーが悲しむため、質問に対してまずは返信をしたい。
質問内容を読み、これは修正が必要だと考えた場合でも、基本的には修正前にコミュニケーションをとるべき。
質問はあくまで「質問」であり、「修正依頼」ではない。

もしも「なんでこの実装にしたの?(あり得ないでしょ)」という趣旨のコメントを残すレビュワーがいたら、それはレビュワーがよろしくない。

11. 後で対応するものはタスク化しておく

「本PRで対応しなくても良いですが、このように変更したいですね」というコメントがつくことも多い。
PRに関係あるコメントであれば対応したいが、本筋から外れたコメントであれば別途対応とするほうが健全。
本筋から外れたものを色々と対応してしまうと、PR肥大化問題に繋がるため。

そういったものは、別途対応すると合意を取り、しっかりとタスク化・Issue化しておく。
タスク化しないと忘れ去られてしまうため。

マージ :tada:

レビューが完了したらマージ!
以上!

おわりに

まれによく観測される「レビュイーの手間は減っているが、レビュワーにそのしわ寄せが来る(もしくはその逆)」というケースを改善する手助けになれば嬉しい。

0
0
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
0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?