なんでもいいからLGTM以外のコメントをしよう
概要
エンジニアである以上、日常の業務においてコードレビューは避けて通れないものです。しかし、レビュー依頼を受けて、さっと目を通してApproveするだけになってしまっていることは多くないでしょうか。
いわゆる「素通し」になっているコードレビュープロセスに対して、自分なり検討したことを本記事で述べていきます。
ケース別の対処
自分自身の「とりあえずApproveしてしまった」というケースを主軸として振り返ってみると、主に以下の3つの要因が考えられます。
- 正直理解できない
- 自分より知識のある人が実装している（だからあっているだろうという先入観がある）
- レビューの時間を取ることができない
これらを一つずつ掘り下げて、ケースごとにどう行動するとよいか考えてみます。
1. 正直理解できない
理解できない部分があることは、どんなエンジニアにも起こり得ます。
それを恥ずかしいと思わないことが大切です。
自分のスキルセットが不足していて理解できない場合
「この行は何をしている処理ですか？」
「この処理を追加した理由はなんですか？」
と、素直にコメントで聞いてしまえばいいのです。聞きにくければ「勉強不足で申し訳ないのですが」という言葉を添えれば、角も立ちません。
これには2つのメリットがあります。
一つは、単純に自分の知識が増えることです。
もう一つは、実装者が説明のためにコードを見直した際、「あれ、これもっと簡単に書けるな」や「あ、バグだ」と気づくきっかけになることです。
実装者側（レビュイー）は当たり前に実装したつもりでも、案外見落としていることが多く、このような素朴な質問で気づかされることも少なくありません。
スキルを高めるという意味では、同じPRに対して自分よりシニアなエンジニアはどのようなコメントをしているかを観察してみるとよいです。とくに、自分にはまったくなかった視点から見ているコメントをみてレビューに対する新たな視点・観点を取り入れることができます。
PRで何を実現しようとしているのか理解できない場合
これは実装者の説明不足である可能性が高いです。
チケットのリンクがない、背景の説明がない状態でコードだけ渡されても、正しく実装できているのかどうかを判断できるはずがありません。
「背景がわからないので判断できません。チケットのリンクを貼るか、概要を追記してくれませんか？」などと差し戻してしまって構いません。 私自身もレビュアーとして、この指摘をよく行います。
変更量が多すぎて、本質的な変更が理解できない場合
「読む気が失せる」というのは、人間の正常な防衛本能です。何百行もの変更の中に、重要なロジック変更と単なるリネームが混ざっていたら、誰だって見落とします。見落としを防ぐためには適切な粒度でPRを作成して貰う必要があります。
なので、無理して全部読むのではなく、「PRを適切な単位で分割してください」と指摘することで、立派にレビュアーの役割を果たすことができます。
2. 自分より知識がある人が実装している
「あの人が書いたんだから間違いない」というバイアスは、今すぐ捨てたほうがいいでしょう。どんな凄腕エンジニアでも、疲れていればタイポもしますし、不要なコードを消し忘れることもあります。
むしろ、高度すぎる複雑さを盛り込んでしまうこともあります。自分が読んで「難しい」と感じたなら、そこには「自分の知識不足」なのか「一般的なエンジニアレベルとして難しい」のかという2つの可能性が含まれています。
もし後者であれば運用負債となりえることなので、「もう少し読みやすく書けませんか？」という指摘をしてみてください。将来そのコードをメンテナンスするチーム全員を救うことになるかもしれません。
3. レビューの時間を取ることができない
※「適切な状態でレビュー依頼されている状況である」と仮定します。
コードレビューを「雑務」だと捉えているから、後回しになってしまうのではないでしょうか。
コードレビューとは、一定の品質を担保するための立派な業務です。まずは捉え方を変えるべきです。
では、他にどういう障害があり得るでしょうか。
それは「コンテキストスイッチのコスト」ではないかと思います。あるタスクAに集中しているのに、横槍的にレビュー依頼がやってきて、脳を切り替えなければならないという状況です。
これに対しては、レビューをする時間を決めてしまうことが手っ取り早い解決策です。私は、急ぎのものでなければ朝一番に実施しています。
優先度をあげるという以下記事も参考になります。
レビューを出す側として心がけること
一方で、コードレビューを出す側の立場としては、以下のプロセスを経ると良いと考えています。
- タスクまたは実装を細かく分解し、できる限り1つのPRの変更行数を減らす
- 自己レビューを行い、その意図をコメントとして残す
- 「一番理解しているのは実装している自分である」という矜持を持つ
1. タスクまたは実装を細かく分解
レビュアーにとって、差分は少なければ少ないほど嬉しいものです。心理的ハードルが下がり、結果として早くレビューが返ってきます。
巨大なPRを1つ出すより、小さなPRを3回出すほうが、トータルのマージ時間は早くなることが多いです。特にはじめのうちは「これは分けすぎかな？」と思うくらいでちょうどいいでしょう。機能追加とリファクタリングを分ける、設定ファイルの変更だけ分ける、といった工夫だけで、レビュアーの動きは大きく変わります。
2. 自己レビュー
PRを作成してすぐにレビュー依頼を出してはいけません。自分ですべきことがあります。
まずはGitHubの「Files changed」タブで変更内容を確認してみましょう。そうすると「あ、消し忘れたデバッグコードがある」「この変数名、微妙だな」と気づくことが意外と多いはずです。
次に、「自分でもこの実装は少し気になっている」「一見こう見えるけれど、構造上仕方ない」といった、実装時に考えたことがあるはずです。それはコードの表面には出てきませんので、「ここは〇〇という理由でこう書いています」とあらかじめコメントしておきましょう。
このコメントがあることで、レビュアーの納得感は上がります。また、もし前提に勘違いがあれば指摘しやすくなりますし、より良い実装方法を提案してもらえる確率も高まります。
最後にDescriptionがわかりやすく記載されているかを確認しましょう。特に背景や目的は重要です。また、実装が目的を達成している証跡（テスト結果など）があると、レビュアーからの信頼も得られます。
3. 一番理解しているのは実装している自分であるという矜持
ここだけ少し感情論になりますが、気持ちの持ちようは重要です。
「たぶん動くと思います」ではなく、「このコードは責任をもって本番に出せます」という顔でPRを出しましょう。
「レビューでダメなところは指摘してもらえるだろう」という甘い考えで出されたPRと、自信を持って出されたPRとでは、実装の端々に違いが現れてきます（この違いは経験上ドキュメントの場合に顕著に現れます）。
レビュアーはあくまで品質を担保するための機構の一つであり、自分自身がその実装の「第一の理解者」であるべきです。
現代のレビュー事情
ツールの進歩
CIやAIが進歩している現代においては、レビュイー・レビュアー双方の負担を軽減できる策がいくつも存在します。
- LinterをCIで実行し、文法チェックを自動化する
- ポリシーを明文化し、PolicyAgentなどをCIで実行して組織ルールの遵守を自動化する
- Claude Code や GitHub Actions などのAIレビューを導入する
Linterは導入コストも大きくないので、入れておいて損はありません。また、自己レビューとしてAIを活用することで、コストを削減することも可能です。
もちろん、コストやセキュリティの観点で導入が難しい組織もあるでしょう。ツールで全体的なコストを下げることはできますが、最終的には「人間のレビューを通すことで担保できる品質」が必ずある、というのが私の考える現代のレビュー事情です。
AIの進歩とレビュー負荷の増加
上述の通り、ツール活用でコストを下げられる余地がある一方で、AIによる実装が進むことで、皮肉にもレビューコスト自体が上がっている側面もあります。
- まとまった大きなPRが上がりやすくなった
- 「意図はわからないけれど、動くからPRを出してしまおう」というケース
- 「AIが言うんだから間違いないだろう」という思い込み
こうした状況では、人間がレビューすべきことと、自動チェックすべきことを分離して考える必要があります。さらには、コードレビューよりも前の段階で、設計や実装方針をレビューすることの重要性が高まっていると思われます。
それでも、実装レビューの段階で初めてわかることもあります。そんな時、「わからないことにはわからない」「悩んだことには悩んだ」と自身の思考履歴を言語化して記録に残すことが肝要でしょう。
まとめ
「わからない」と言うのは怖くありません。「間違っているかも」と指摘するのは失礼ではありません。
思考停止でApproveボタンを押すのをやめて、一言でもいいからコメントを入れることから始めてみましょう。
その一歩は、自身の成長にもチームの成長にも、必ずつながるはずです。