00:01
こんにちは、Riddleです。
こんにちは、ひびのです。
コードレビューの概要
今日のお題は、プルリクのレビューで気をつけていること。
プルリクエスト、GitHubだとプルリクエストで、GitLabだとMerge Request、
他のやつだとまた別の呼び方があるかもしれませんが、
簡単に言うとコードレビューですね。
そうですね。
コードレビューで気をつけていることなんですけれども、
最近シュワシステムさんからですね、
Look Good To Me みんなのコードレビューという本が出版されました。
LGTMを略さない。
そう、LGTMというレビューを承認してもらうときに、
なぜかついてくる謎の4文字の単語、LGTM。
最初見たとき、何やねんこれって思った方も多いじゃないでしょうか。
僕も最初そう思いました。
はい、私もそう思いました。そのボタンコとして使ってません。
はい、ということで、この本がどんな本かを簡単に説明すると、
レビューとは何ぞやみたいなことが最初に書かれていて、
こういうふうにレビューするといいよっていうテクニックの紹介と、
あとはレビューをチームで導入するために、
こういう形で導入すると効果出やすいよ、
とかチームメンバーに納得してもらいやすいよ。
あとはレビューじゃなくて、ペアプロとかモブプロっていう、
一人でプログラミングするんじゃなくて、
慣れてる人と二人でプログラミングするとか、
チームみんなで一つの画面を共有しながらプログラミングするっていう方法があるんですけれども、
それとの使い方の違いみたいなことが書かれていたり、
一番最後は今話題のAIですね。
AIを使ったレビューがどんな感じなのか。
これちょっと本当に今収録している2025年の6月26日の最新のものではもちろんないので、
ちょっと古い情報にはなるんですけれども、
温度感というか、感どころをつかむ意味では有用かなという内容でした。
でも本当にコードレビューに関するトピックが一通り網羅されてそうですね。
そうですね。なのでこの本の対象読者としては、
あんまりレビューをしたことがないよとか、
これからレビューっていう文化を取り入れたいっていうチームとかには結構有用だと思っています。
一方で既にレビューをガンガンやってますとか、
レビューすることは当たり前だよねっていうカルチャーのところには、
そこまで、改めて自分たちで言語化するときに役に立つかもっていうぐらいですかね。
そうですね。それで言うと、僕はこれまでの職場だともう当たり前にレビューが導入されている文化だった。
で、自分もそれを疑問に思わず、何を教えられるわけでもなくレビューを始めたので、
そういう体系的なレビューの知識みたいになっている本の内容はやっぱり気になりますね。
本当ですか?
はい。
じゃあこの本をベースに、我々の経験も含めて、
どういうふうにプリックだったりだとか、
コードのレビューをしていくと良いんだろうかみたいな話をしていければと思います。
よろしくお願いします。
レビューの必要性
まず前提なんですけど、そもそも何でコードレビューっていうのが必要なのかみたいな話をしましょう。
この本に書かれていたのは大きく分けて3つあります。
1つ目がレビューによってバグとかセキュリティ的な問題が減少するってことが論文とかで確認されているんですね。
例えばテストは通ってるけど、そもそもこれ使用満たしてなくないとか、間違えてないみたいな、
そういう問題だったりだとか、セキュリティ的な問題がそもそも紛れ込んでるとか、
そういうものがあると、それを本番に出してしまうと大きな問題につながってしまう可能性があるので、
それを事前に潰しておこうみたいな、よくあるやつですよね。
そうですね。
2つ目が明快さと過読性の向上とか、中長期のメンテナンス性の向上っていう概念なんですけれども、
個人開発じゃない限り基本的には複数人で開発をしていくことになるので、
そのコードを作ってからずっとサービスが存続する限りは維持していかないといけないと。
チームメンバーって変わるケースが多いので、昔のことを知らない人がコードを触ることが普通です。
なのでその人がちゃんと触りやすいように、どんでこうなってるのかとか、
何か機能を追加するときに追加しやすいような設計になってるのかみたいな観点でレビューをしてもらう必要があるという感じですね。
そうですね、おっしゃる通り。
最後の3つ目がチーム全体の理解の向上みたいなところで、
これは改めて言語化すると確かにって感じなんですけれども、
レビューによってここは実はこういう理由があるのでこうしてくださいっていうコメントよくもらうと思うんですよ。
あとは実はこっちに同じことやってるからそれを使ってくださいとか。
これをだからすごくしっかりした言葉で言うと知識の移転ってこの本では紹介してくれていて、
詳しい人が詳しくない人にこうなんです、このドメインではとかこのプロダクトではっていうことを教えてあげる場としても使えるので、
それがレビューとしてはすごい価値ありますよねっていう3つ目の理由になってます。
それが続いていってシステムを維持していく人、これから維持していく人たちに語りつかれていって知識が伝播していくってことですね。
そうです。
なのでレビューっていうのはこの3つを満たすというか求めてやるものなので基本的には今後も続いていくでしょうっていうのがベースの考え方になっています。
そうですね。
レビューの実践
その上でレビューをやっていくぞってなったらチームの中でどういう風にレビューをやっていこうっていうのをまず擦り合わせないといけないんですよ。
例えば何のレビューをして何はレビューしないのかみたいなラインですね。
すごい分かりやすいやつで言うとなんかタイポってよく言うんですけど、音言の間違えとか簡単な英単語の間違えとか空白のところがタブになってるみたいな。
はい。
挙動に影響ない間違いってあるじゃないですか。
ありますね。
そういうのを人が指摘し始めるとキリがないというか本質的なところに時間を割く余裕がなくなっちゃうのでそういうのはレビューせずにフォーマッターとかリンターとかでちゃんとカバーしてレビューをする前にコードを書いた人がちゃんと直してねみたいな感じにするっていうのが一般的ですよね。
はいはいはいそのレビューに出す前にセルフチェックで確認すべき。
そうですね。
なのでレビューの焦点として考えたいのは例えばファイルがちゃんと正しいディレクトリに置かれてることとかコードが複雑になってないよとか機能がちゃんと実装されて動いてるよねみたいなところだったりテストが網羅されてるよねみたいなところをチームの中でどれが必要なんだっけっていうのを優先順位に決めて用意する必要があると。
これもそんな違和感はないんですけど個人的にはガチガチにやりすぎるとルール守んなかった時に書いたのになんで守んないんだみたいな話になってめんどくさいんであんまりガチガチにルール決めない方がいいと思います。
まあそうですねあと個人的にはさっきの話だといわゆるタイポーはフォーマッターで防ぎきれなくないみたいなことも。
自分はVSコード使ってるんですけどVSコードではタイポーを教えてくれる拡張があってタイポーがあるとも教えてくれる。この英単語おかしいよみたいな。
知らなかった。
でもこの拡張機能の厄介なところは我々別に常日頃から全部正しい単語を使うわけじゃないじゃないですか。
そうですねドメインによった単語だったりあと何なら日本語をそのままアルファベットで表現するみたいなこともあるはありますもんね。
あと言語によりますけど上の方に外部ライブラリをインポーズするときとかでGoとかだとGitHubのURLみたいなのを書くんですけどGitHubのURLにユーザーの名前とか入ってるとそれも対象になるんですよ。
それこそ英単語にないアルファベットの羅列のことが多いですもんね。
僕のLIR、LIAみたいな単語とか存在しないんでそれ毎回エラー扱いされるんですよ。
それは鬱陶しいですね。
だからそれを見つけるたびに僕はそれを自前のリストに追加して、辞書ですね辞書に追加してこれはチェックしなくていいよみたいなやつがあるんですけどこの前この辞書を見たら500件ぐらい追加されてて。
これまでの開発で鬱陶しいと思った歴史ですね。
あとGoとかだとちょっと単語をなるべく短縮することが美徳とされるんですよ。
はいはいはいそうですよねそのローカルの変数は本当に頭文字だけつなげて3文字くらいの変数にするとかありますよね。
だからAMMとか謎の単語が生まれるんですけど絶対選べるんでAMMを登録するとかいう意味わかんない不毛な作業をするんですよ。
それはGoと相性悪いリンターですね。
まあまあそうですね。
なので一長一短なんですけどそういうテクニックもあります。
で他にもレビューがマージされるまでアップルーブかなアップルーブされる承認されるまでにチームとして期待する時間みたいなのをすり合わせると良いよという話もありました。
ああなるほどいわゆるそのレビュー待ちの時間をなるべく減らそうみたいなムーブメントも必要ってことですね。
コードレビューの時間管理
どっちかっていうと時間を減らしたいのはそうなんですけれどもこの本では例えば複数の国とかでまたがって開発してるような場合にやっぱり日付をまたいでしまうみたいなこととか要はあるわけですよ。
ってなった時に本当だったら2時間ぐらいで見れるはずなのに国をまたいでゆえに翌朝になったりとかするんで大体このプロジェクトでは1日以内に見れるようにしましょうみたいなところをちゃんと規定してあげてそれを超えたら
プルリクを作った人がレビューワーに対して何かアクションをとる何か困ってますかとか何かこう手伝うことありますかみたいなきっかけにできるってところで1個決めておくとやりやすくなるよということがありました。
なるほど確かにプルリクがなかなかレビューしてもらえないその最速とかも人間のコミュニケーションだしそのあたりもしっかりルール化しておくとみんな遠慮なく伝えられるよねみたいな感じですかね。
そうですね最低限これぐらいの時間は待ってからやるよみたいな感じ。これは結構いいなと思いましたね。
あとは心構えの話なんですけどこれはちょっと僕の意見が多分に含まれるんですけどあなたがもしレビューワーだったとしましょうとレビューワーだとするとあなたはこの変更に対して承認をしたということですね。
承認とかはひめのさん結婚してると思いますんで契約書交わしておくじゃないですか結婚。
そうですね婚姻届。
結婚式とかでもしやってる場合なんかカトリック系のやつだったら前に行って外人が拙い日本語で辞めるときも健やかなるときもみたいな互いに愛し支えることを誓いますかみたいな。
あれも承認してるからっていう感じで約束を守るというか私が認めましたっていう重いやつなんで結局そのレビューをして承認したということはあなたはこの変更に対してokと言いましたよね。
バグがないですよね使用通り動きますよねっていうことをちゃんと確認した上でokって言いましたよねっていうことに同意したわけですから。
それはその本ではアプローブとはそういうものだっていうことが書かれているっていうことですか。
これは僕が勝手に言ってます。
なるほど。
そういう気概を持ってやる必要があるかどうかっていうのはちょっと怪しいですけど哲学的な話として。
自分のアプローブにも責任を持てってことですね。
レビューの心構え
僕もねノールックマーチとかしますけど。
ケースはケースなんですよだからそのこの変更がやって別に問題あったら簡単に切り戻せるやつだったら別にインパクトないからアプローブしてもいいけど影響が大きいやつとかはもうめちゃくちゃしっかり見るしそこらへんはちょっとさじ加減なんですけど
根底にはこういう心構えあったらいいんじゃないかなっていう試験です。
特にクリティカルなプルリクを見るときは大切ですね。
実際にレビューするときどうやってやんねんっていう具体的な方法なんですけど
まず前提としてプルリクが出てますと。
いろんなCIのテストが走ってると思うんですけどそのテストが全部通ってることは確認してくださいと。
もうコケてたらもう突き返してください。
コケてるぞと通ってからこっちに話し通しこいよと。
そうですねそこは最低限プルリクを出した側でまず通してから話を持ってこいっていう感じですね。
あとプルリクが汚かったらこれ言う汚いは修正範囲が広すぎるとか。
理由なく大量のファイルを1プルリクで編集しててあまりにも見るのにコストがかかるみたいなケースは分割してくださいっていうのを言いましょう。
それは大切ですね。いわゆるレビュアーの認知のしやすさ重視というか。
こっちも承認というリスクを背負うんで承認しやすいようなプルリクにしてくれっていう権利もあるんで恐れず言ってください。
相手に承認という重い行為をさせるんだからこっちもそのつもりでプルリクを出そうっていう思想ですね。
これは本に書いてないです。僕が書いてるんです。
あと前提としてプルリクはこういう観点でレビューしましょうねっていうのはチームで決まってるならまずそれを基本的には守りましょう。
だからこの観点は見なくていいよってなったらチームで決めたならその観点は見ない。
なるほど。これまでだと僕ここは見ないみたいなことを明確に決めて運用してるチームって多分なかったなと今思う。
そうですね。そんなことあんまりないんですけど。例えばですよ。例えば今とにかく作り切らないといけないと。
一旦拡張性とかはめーつぶろ。同じ処理が何度か登場しててももういい。一旦もういい。
ドライの原則何も即してないけどドライっていうのはドントリピートイットかな。
そうですね。開発現場では往々にして一旦このコンスプリントだけはデリバリー重視でマージさせてくれみたいなタイミングもあるはありますもんね。
いろいろあると思うんですよ。状況によってね。あんまり状況によって変えるのも良くないんですけどいろんな都合があると思うんで。
なのでその観点見ないって決めたなら見ない。
実際にどういう順番で見ていくのがいいんだろうっていう話ですよね。レビューするとき。
これはもう本に書いてないですよ。これも完全に試験ですけど見やすい順に見ていくっていうのが一番だと思ってます。
見やすい順。
一つ目。テストから見る。
そうですね。確かにテストを自分の加えた変更に対する使用がしっかり網羅されているというか。
そうですね。テストを見たら何の変更をやっているのか。もちろんプルリクのサムライズとか内容でわかりますけど
テストを見るとちゃんとテストが書かれていればそのテストの修正内容がやったことなんでそれが満たされているかどうかとか
そのテストが仕様と同理なのかっていうまずチェックした上で進められる。
確かにそれは一理ありますね。
二個目。処理の先頭から見るですね。
処理の先頭。それこそ例えばAPIの実装だったら最初にAPIのハンドラーから見るみたいな感じですかね。
そうです。APIのハンドラー見て、ユースケース見て、サービス見て、ドメイン見て、リポジトリ見て、データベース操作見てとかそういう流れ。
それもわかりますね。
最後。見やすいファイルから見る。
見やすいファイル。見やすいファイル。もちろんいろいろありますよね。
例えばファイルの長さだったりとか、あとは自分が触り慣れているファイルとか。
そうですね。単一の関数を追加してて、それに対するユニットテストがあって、それを外部から読んでるみたいなときはとりあえず単一の関数だけ見て、
この関数はOK、テストもOK、じゃあこれとこれはチェック、で次のやつ見るみたいな感じで端っこから攻めていく。
それもわかるな。特に長いプルリクだと自分も自然とそう見ている感じがします。
レビュー実施の実践
そうですね。なので、別に見る順番に正解はないので、自分が一番見やすいなっていう方法を見つけて、これは別にプルリクごとに変えてもいいので、
一番自分が時間もかからずかつちゃんと見れるっていう見方をアレンジして生み出す、考える、慣れるですね。
そうですね。それこそその辺りの感覚はレビューを何回もやって掴んでいくしかないものですよね。
そうですね。なので、もし皆さんが新規のプロジェクトにの参入者だったりだとか、そもそも長くはいるけど経験がまだ足りなくてレビューができないよっていう人絶対いると思うんですよ。
自分もそうでしたけど。その時はもうね、これは慣れるしかないんですよ。経験を積んで。
おっしゃる通り。
レビューの仕方も言語に対する理解もドメイン知識も全部。なので、その場合は上長やチームと相談して、アップルーブはしないけどプルリクには目を通すという役割をもらってください。
はいはいはい。ありますね。僕が新卒で配属されたチームでも、アップルーブまでできなくてもいいからプルリクしっかり見て、わからないところはコメントでしっかり聞いておけみたいなことは最初に言われましたね。
そうですね。さっき知識の転移もレビューの一環という話あったんで、あなたがわかんないことは次入ってきた他の人もわかんないので聞いてください。
そうですね。それは本当に大事。あと聞いておけばそれこそ後々検索で引っかかって、レビューを返さなくても知識として伝播していくチームのためになるという良い側面もありますもんね。
そうです。なので、もし聞いてくれてる人でレビューまだ全然慣れてないよという人はこれやってください。確実です。もうこれしかない。
大事。
AIとか言ってる場合じゃない。
まあまあまあ。それに言うと僕最近プルリクのレビューを見ることあるじゃないですか。コパイロットにも一応見てもらうみたいなことはやってますね。
そうですね。これは次話そうと思ってたAIとの付き合い方、過去レビュー編の話になるんですけども。
今、巷にたくさんレビューしてくれるAIツールありますよね。コパイロットの今おっしゃっていただいたプルリクもそうですし、クロードのコードのGitHubアクションとか。
はいはいはい。ありますね。
あとは昔からあるCoreTravitとかAIReviewerみたいなツールがあると思うんですけど、どれも人間の代替にはまだなり得ないが強力なツールであることは間違いないという状況でした。
AIとの効果的なレビュー
本当にリポジトリの全体的な理解とか、あといわゆる設計のコンセプト、ドメインのコンセプトみたいなところの理解は確かに浅いんですよね。まだまだ。
そうですね。いずれそのレベルになるかもしれないんですけど、一旦今はそんなことないっていうのが実情です。
なので個人的にうまい付き合い方としては、人間にレビューをする前にAIにレビュー依頼して、そのフィードバックを受けてプルリクを修正して、人間のコードレビューに進むっていうのが一番いいと思います。
確かにそれはいいかも。特にDiffが出ている箇所については、本当にしっかりベストプラクティスを最近は指摘してくれるなっていう印象なので、それを経てから人間に読んでもらうアリですね。
逆にこれをやらずに、人間のレビューを挟まずAIだけで片付けようとすると、どうなるかというとレビュワーとしての能力がめちゃくちゃ下がるんですよね。
具体的には批判的な思考って本では紹介されてたんですけど、人の行動を見てそれがいいのか悪いのかとか、改善すべきなのかってやっぱり自分の中のノウハウベースになるんですよね。
そうですね。
そうすると、それがやらなくなったらその能力つかないしなくなっていくんで、それはめっちゃ困るんですよ。最終的に責任等の人間だから。
そうですね。おっしゃる通り。
なので、AIレビューとのうまい付き合い方は、現段階としては最初の利用を自分でやって、その後のしっかりとした人間のレビューはちゃんと人がやる。
はいはいはい。それこそプログラミング言語に詳しければ誰でもできるような表層的なレビューはもうAIに任せる。一方で、業務知識とかもう少し深い経験が必要なところを人間が担保するみたいな使い分けですね。
そうですね。あと追加の使い方で、その後人間がレビューすると思うんですけど、人間がレビューした後にこういうふうにしてほしいとか書くじゃないですか。
なぜならこういう理由だから。
そうですね。
優しい人がそこに参考実装とか載せてくれません?
ああ、はいはい。僕もそれはやるようにしてます。
でも、ボリュームによりますけど手間かかるじゃないですか。
そうですね。
そこをレビュー側がAIエージェントに依頼して、こういうふうな修正をするといいと思います。
で、AIエージェントをGitHubなスラッシュコマンドとか呼び出して、上記を満たすような修正をサジェスションとして出してみたいな感じにすると、そこを書いてくれるからレビュー側の手間がちょっと減るんですよね。
ああ、確かに。それはなかなかレビュー側としてもうまい付き合い方ですね。
うん。ということで、AIエージェントとの付き合い方を紹介しました。
レビューのコメントの重要性
あと細かいテクニックですけど、いろんなコメントすると思うんですけども、そのコメントの種類ってあるじゃないですか。
これは絶対直してほしい。これは思いつきだったりだとか自分の考えです。もしくはこれはただの質問ですみたいなジャンルがあると思うんですよ。
ありますね。
そのジャンルをよくやる方法としては、MustとかNitsとかShouldとかAskとか、そういうNitsはなんだろうね。
ありますね、Nitsは。
NitsってNitsって言うんですか?
NitsはNitsPickみたいな英単語があって、細かいところですがみたいな。
ああ、そうなんですね。
よく使うのだとIMO、In My Opinion、個人的な意見だけどみたいな意図で。
For Your Informationとかね。
まあでも、いわゆるレビューに対する緊急性というか、セビリティみたいなところをそこから指せるのはだいぶありがたいですよね。
そうですね。なのでレビューをする人は自分のコメントがどういうセビリティレベル感なのかを、そういうものをつけておいてあげることで相手はすごい理解できる。
これはスルーしてもいいのかとか、絶対対応しないといけないのかとかわかるので、そういうのをつけてあげて相手の解釈の時間を減らしてあげるっていうのはめっちゃ大事なので、それもぜひやってみてください。
そうですね。本当にコード越しのコミュニケーションって感じですね。
はい。そんな感じで今回はGitHubとかGitLab、もしくは全般的なコードレビューに対する方法だったりコツみたいなところを紹介しました。
より詳しくは今回使ったLooks Good to Meというシューバーシステムさんの本を読んでいただければと思います。
またこの動画の感想などはハッシュタグURLITのほうで募集しておりますので、ぜひコメントください。
またエピソードの概要欄にGoogleフォームのリンクもありますので、そちらからの投稿も大歓迎です。
ありがとうございました。
ありがとうございました。
