メンバー対談「レビュワーとしてレビューするときってどんな感じ？」yokota × suzuki 編（前編）

yokotaじゃあいきなりyuri（注：suzukiのハンドルネーム／普段からHNで呼び合っています）に質問だけど、自分が関わるプロジェクトのレビュワーになったとき、どういうことが大変って思う？

suzuki大変？ レビューで大変なところか…。

yokotaどんな感じだとレビューしやすいとか、しづらいとか、そういうことでも。

suzukiそうですねぇ、レビューしやすい感じなのは、まず、小さいこと？

yokotaああ、プルリクの粒度とかが？

suzukiそうそう。

yokotaボリュームは重要だよね。

suzukiそう、このプルリクがどうなったらOKっていう確認事項が少なければ少ないほど楽だとは思う。

yokotaああ〜、Approvedできる条件が明確ってことだよね。

suzukiそうそう、分けられるものであれば条件が分けられてたほうが。足したら見る総量としては変わらなくても、コンテキストが積まれてしまうとそれぞれを見る範囲が広くなってしまうから。

yokotaうんうん。

suzukiあとはそうだなぁ、これはアリなのかわからないけど、UIの再現がノールックで済むのならめっちゃ早いと思う。実装の妥当性だけを見ればいいなら早い。

yokotaああ〜、コードの妥当性だけを見ればよくて、UIの再現性についてはチェックする必要がない状態ってことだよね。プルリクを作成した人がきちんと担保できていて。

suzukiそう、見る範囲がコードだけでよければ、すごく楽。

yokotaうん、あるね。

yokota見た目を再現できてるかから確認しないといけないって、しんどいよね。レビュアーがすべてのブラウザで確認をしないといけないってことだしね。作業としてはだいぶコストだよね。

suzukiどうなんだろう、そこは見ないですって言えばいいだけの話なのかっていうのが、正直わからないね。そこは（コードレビューでは）見ないんで、（再現は作業者の責任範囲として）お願いしますって言えば済むのかどうか。

yokota私はプルリクに該当箇所のスクリーンショットが貼ってあるだけで、パッと判断できることが多くなるから、スクリーンショット貼られてるか貼られてないかだけでだいぶ変わるなって思うかな。

suzuki確かに。どこの話をしているのか自体もわかりやすくなるしね。

yokotaそうそうそう、URLとかリファレンスとかStoryBookのリンクが有るとか、そういうのも当たり前にあってほしいんだけど、そこにスクリーンショットがあるかないかで、レビューのしやすさが違う。

suzukiそうだね。

yokotaあとこないだのKPTで言ってたけど、「なんでこういう実装にしたんだっけ？」みたいなToDoとかノートコメントが残ってるだけで、レビュー通しやすくなる気がする。

suzuki確かに。

yokotaそのプルリクの中で見てほしいこととか、チェックの優先度が高いことがはっきりしていると、「あっここはまだ見た目が再現されていないけど、後で別のプルリクで対応するのね」みたいなことがわかる。それがないと、どこまでチェックすればいいのかわからなくて、こちらから質問を投げ返したりして往復が発生するから、時間がかかる。

suzuki確かに、それはある。

yokotayuri氏はプルリク見るとき、どこから見てる？ コミットログから見てる？ いきなりChagned File から見てる？

suzukiあー、そうですね、わりと差分を先にバーって見ちゃいます。差分見て「ん？」っと思ったところがあったら実装を見に行きます。コミットログはざっとだけ見て、わりとすぐ差分のほうを見ますね。

yokota同じくだ。

suzukiわりとそのパターンが多い。

yokotaうんうん。

suzukiあと、UIに関連するプルリクだったら、まずローカルで立ち上げますね。ローカルで見る。

yokotaああ、そうだね。それは自分のローカルでも再現するよね。

suzukiでもわりと見た目を作るとき以外のレビューでも、自分のローカルでは見るかな。たまにコミット積み忘れとかがたまにあるんで、どんなときでも一応ローカルでは見ます。

yokotaあるある、あるね。

suzukiUIの再現が含まれるプルリクだったらしっかり見るし、そういうのでなければ、必要要素ちゃんと出てるかなってところざっとみて、コードレビューして、終わり。

yokotaあー、でも私も必ずローカルに持ってきてるな。そこでエラーに気付くこととかもできるし。

suzukiうん。

yokotaCSSの中とか見てる？

suzukiCSS、うーん、ざっと見る。

yokotaざっと見てる？

suzukiうん、ざっと。細かくは見てない。

yokotaうんうん、まぁね、崩れてたりしたら見るけどね。ちゃんと再現されてるんだったら、ある程度は作業者に任せてたりするけど、変数のところはちょっとしっかり見るかも。変数であるべきところにちゃんと変数が使われてるかなぁとか。

suzuki確かにね。

yokotaあと、例えば、なぜかbefore要素が書かれてて、それなんで必要なのって聞いたら、別の場所から移植されてきただけで必要のない要素だった、みたいなことがあって。そういうのに気付けたりはするから、ざっとは見たいよね。コーティングルールに沿ってるかも見たいし。

suzukiそうそう、命名とかは絶対に見るよね。そうすると結局見る箇所って多いんだよね。だからプルリクは小さいほうがうれしいってなる。