コードレビュー「LGTM!」は人によってバラバラ?

に公開

はじめに

こんにちは!
SREホールディングス株式会社のエンジニアをしております、クマカワです。

常日頃コードレビューをしている皆様、何をもって「LGTM」していますか?

ご自身の「LGTM」にどのような意味を持たせていますか?

「完璧なコードですよ」
「バグはなさそう」
「(バグの有無は知らないけど)見たよ」
「(ちょっとコード汚いけど)動くのでいいと思う」

人によって様々だと思います。

この記事では、
簡単なことだけど意外とできてないのでは?というコードレビュー改善案など、
Googleのコードレビューガイドラインも参考にしながら、
個人的な見解で記述させていただこうと思います。
Google Engineering Practices Documentation

...コードレビューの正解については言及しません。
残念ながらそこまで辿り着いておりません故、ご容赦ください。

※以下、レビュー手段はGithubにおけるPR(Pull Request)と記述させていただきます
※Reviewerとrevieweeをカタカナで表す場合、レビュアーやレビューアなどパターンあるかと存じますが、ここではレビュワーとレビュイーとさせていただきます

コードレビューエピソード① 説明のないPR

詳細

PRの説明欄、活用できていますか?
次のようなPRを見たことはないでしょうか。

例えば、SlackやChatworkなどGithubとは異なるツールにてレビューを依頼する際。

レビュワーはメッセージを受け、PRを確認しに行きます。
開くと、説明欄に何も書かれていない…!

PRの目的が分からないと、レビュー観点も定まらずレビュワーは困ってしまいます。
何を確認したらLGTMを送ればいいのでしょう?

依頼時にチャットで簡単に書いたからいいじゃん!とも言われそうですが、
そのPRがマージされてから数ヶ月後、
とある変更が入ったのがいつなのか、どのような背景で変更が入ったのかを確認したい時に物言わぬPRだと困りますよね。
わざわざ過去のチャットを遡って探すのは手間です。

変更本人に確認可能ならばその場では問題ないですが、
既にプロジェクトメンバーから外れていた場合、誰も知らない永遠の謎だけが残ります。

対処

当然の如く活用されているかもしれませんが、.github/pull_request_template.mdを使えばPR作成時にどんな説明を書くべきか、チームメンバーで認識を合わせることができます。

リポジトリ用のプルリクエストテンプレートの作成

こちらWriting good CL descriptionsにもほぼ1ページ分記載されていました。

What change is being made? This should summarize the major changes such that readers have a sense of what is being changed without needing to read the entire CL.

Why are these changes being made? What contexts did you have as an author when making this change? Were there decisions you made that aren’t reflected in the source code? etc.

何を変更したのか、なぜ変更したのか。記載しましょうという内容です。
せっかくですから読んでみてください。

A well-written CL description will help those future engineers – sometimes, including yourself!

ちゃんとPR説明欄を記載すれば、将来他のエンジニアもしくはあなたの役に立ちますよ!と記載されていますね。

現在私が参画しているプロジェクトでは、以下の項目をテンプレートに含んでいます。

  • チケット/背景
  • 概要
  • 動作確認
  • Rv依頼前チェックリスト

それでも記載漏れがあったり、記載しているのに読まれなかったりもありますが、、
最低限、PRを見れば何かしら情報が得られるようになっています。

コードレビューエピソード② セルフレビューしてないPR

詳細

レビュイーのマスト作業、セルフレビュー。
確認しないままレビュー依頼出してしまうこと、意外とありませんか?

レビュワーとしては簡単なミスを見つけると、そのPR全体の品質に疑念を抱く場合があります。
「ちゃんと見直してる?他にも似たようなミスないだろうか?」
「動作確認したんだろうか?」
簡単なミスに気を取られて、気がつくべき問題を見落とす場合もあります。

対処

これはもう、多忙な中でも、セルフレビュー徹底しましょう!に尽きます。
当たり前のことですが、マルチタスクしていると意外と忘れてしまうことがあります。
でも徹底!

小さなミスでレビュワーの時間を浪費しないようにしましょう。

コミュニケーションコストを削減し、マージまでのリードタイムを短くできる可能性が上がります。
Github CopilotにPRレビューをお願いするのも非常に有効な手ですね。

The CL author’s guide to getting through code reviewには「セルフレビューしよう!」とは記載はなかったです。
当たり前すぎて記載されてないのかも…
Review the description before submitting the CL説明欄を見直そう!は書いてありましたが)

私も気をつけます。

コードレビューエピソード③ その指摘コメント、修正対応すべき?

詳細

永遠に終わらないPR上の会話を見かけたことはありますか?
例えば、仕様が決まりきっておらず、PRレビュー時に複数人で検討しているケース。

実装しているうちに浮上した問題などもあるでしょうから、チーム内で再確認する必要があります。

早々に対面/リモートでの会話に切り替えた方がいいです。

そして、決定事項を文字で残すことも忘れずに。 ⬅︎ 大事!

また、書き方・好みの問題の会話もがなされることがあります。
この場合の決着点はどこになるでしょうか?

例えば、PR上のコメントで、レビュワーからこんなコメントがあったとします。

レビュイーは対応すべきでしょうか?

対処

キャプチャのような指摘では、なぜその書き方の方がいいのか、説明が全くありません。
レビュワーは、指摘と理由をセットにすべきです。

すぐ会話できる状況ならいいのですが、
文字ベースのやり取りですので、一回でレビュイーに意図を伝える工夫が必要です。

The Standard of Code Reviewに記載があるように、
完璧ではなくても、仕様に沿ってちゃんと動くのであれば、承認しようね!とあります。
大抵の開発の目的は、綺麗にコーディングすることではなく、顧客に成果物を納品することですよね。ある程度の線引きは必要になります。

In general, reviewers should favor approving a CL once it is in a state where it definitely improves the overall code health of the system being worked on, even if the CL isn’t perfect.

That is the senior principle among all of the code review guidelines.

また、コメントに優先度(対応しなくてもいいよ!の意思表示)をつけることも大切です。
改善の余地があることをお知らせするのは自由なので、
「Nit:」をつけて対応優先度が低く、マージ優先してもいいよということも伝えてあげましょう。ということですね。

Reviewers should always feel free to leave comments expressing that something could be better, but if it’s not very important, prefix it with something like “Nit: “ to let the author know that it’s just a point of polish that they could choose to ignore.

Nit:コメントであれば、
レビュイーの知識の幅も広がりつつ、開発もスムースに進められそうです。

個人的な話をすると、自分がレビュイーの場合、コメントをいただくと優先度が低くても、結局修正してしまう傾向があります。
開発をスピーディに進めるためにもある程度のスルースキルも必要かもしれません、、

コードレビューエピソード④  「バグが出ないこと」を担保するか?

詳細

重要な処理を担っていたり、条件分岐が多い箇所のPRとなると、
レビューするのにもやや気合いが入りますよね。

ローカルにそのブランチ落としてきて、
このパターンは?あのパターンは?と動作確認していくと、いつの間にか結構な時間が経っていた、、というご経験ありますでしょうか。

あなたのチームは、コードレビューでどこまで担保するか、チームで認識合わせされていますか?

もちろん、コードレビューですぐ発見できるバグを放置するのは御法度です。
ただそのPRがマージされた後には、テスト工程があるはずです。

コードレビューでバグを全て見つけるために時間をかけると、チームとしての開発スピードを下げてしまいますね。
その後のテスト開始も遅れてしまいます。

対処

The Standard of Code Review

The primary purpose of code review is to make sure that the overall code health of Google’s code base is improving over time.

上記にあるように、コードレビューの目的はバグを0にすることではなく、システム全体のコードの健全性を向上させることです。

あるか分からない発見難易度の高いバグを見つけるために時間をかけるよりは、開発スピードを優先すべき場合もあるのですね。

そして、正しさを担保するためにもちゃんとテストコードを書きましょう!
The Standard of Code Review-Tests

それでもバグは起こり得るものですので、テスト工程で全力で炙り出しましょう。

(雑談)読みたかった本

個人的な話で申し訳ないのですが、
秀和システム新社さんが出版している「Looks Good To Me〜みんなのコードレビュー〜」という本。2025年5月30日に発売されました。

とても読みたかったのですが、私が探した時点(2025.11~12現在)では遅すぎたのか絶版になっており入手できませんでした...😭
https://www.shuwasystem.co.jp/book/9784798071442.html

もし読まれた方がいれば、ご感想お聞きしたいくらいです。

おわりに

皆様、共感できる部分はありましたか?

当たり前なことを書いていますが、
意外とできていない部分もあるのではないでしょうか。

他の開発チームのコードレビューを見てみたいなあと思う今日この頃です。
あなたのチームは、上手にコードレビューできていますか?

セルフレビューなどは、今日から着手できますのでもしまだ改善の余地ありの方は意識してみるとよいと思います。

お読みいただきありがとうございました。

SRE Holdings 株式会社

Discussion