セルフレビューをして自分とレビューする人の無駄な時間を減らそう

CARTA fluct エンジニアのなっかー @konsent_nakka です。

この記事のコンテキスト

「よっしゃPRできた！」「(他の人からの指摘と質問の嵐)」「ぐあああああああ！！」のパターンを何度も踏んできました。

PRのレビューをしている時、またはお願いした時に起きる問題はいくつかあります。

レビュアーに実装の意図が伝わらなくて質問責めにあい、返答したり追加質問されたりしている間に多くの時間がかかる
単純なミスが残っており、無駄にやり取りが往復する
レビューを出した時に返ってくるのが遅い、また自分がレビューする時に時間がかかってしまう

これらの問題はあらかたセルフレビューで改善します。

※レビューする側の話は後日出します。

セルフレビューとは

ここでは

他の人にレビューされる前に、補足説明をコメントしたり、気になっているところをコメントしたり、あたかもレビュアーになりきって自身のPRをレビューすることとします。

ここからは私が意識している流れでセルフレビューの観点について記述していきます。

コードやコメントで表現しきれなかった細かい意図を説明する

まずは自分の中で分かり切っていることを書くところから始めると筆が乗ります。

このコードをなぜこう書いたのか？なんであの関数を使わなかったのか？などをコメントする
- この時、コードのコメントに書いた方がいいものかどうかも合わせて判断する
実装方針を最近変えてレビュアーが混乱しそうなものとか、コードコメントに書くまでもないなーというものは良く書いたりする
- IDはstringではなくintを利用するように全体の設計方針を最近変更した
- テストに利用する正常系データを生成する関数を作ってみた

どうしても理解が難しいコードは補足説明する

これも自分が実装したものは自明で書きやすいので先に書いてます。

レビュー速度を上げるために、理解を助けるコメントを書く
- 自分がレビューする時にこれ読むの辛いだろうなーみたいな処理は絶対どこかである
- 例えば
  - GitHubの表示では分かりづらいけど、hoge関数を追加しただけ
  - bashのコードで$1は第一引数を表していて、全体で使った時はコマンドの第一引数で、関数内で使った場合は関数の第一引数になっている
もちろんそれを踏まえた上で、もっと綺麗に書けることに気づいたりもできる

レビューの観点から「このPRではここまでしか実装していないが、次のPRで対応する」など開発の流れを伝える

レビューのしやすさ、リリースの切り戻しやすさを考えてPRを小さくしたい時に、続きは次のPRでやることをコメントしたりする
- 「これリファクタした方がよくない？」「ちょっとPRが大きいので次に対応しますね」みたいなやりとりを減らせるなら減らした方がいい
そもそも、1つのタスクを実装するのに「N分割して〜〜の順番でやっていく」みたいな時にはPR Descriptionに書いておく
- 最初、または後から視認性が良くなるようにissueにもしっかりと全体像を記載するようにしましょう
- その内容をPR Description用に持ってくるくらいのイメージだと全体が整理されて良いです

納得がいってない設計や命名、意見をもらいたい内容についてコメントで伝える

この辺りから自明ではないので、コメントする時もちょっと筆が遅くなりがちです。筆が乗っている流れを生かしてガーッと書いてしまいましょう。

例えば
- 「ここの命名は今のままだと直感的に伝わらないが、代わりの命名が見つからないので何か思いつくものあったりしますか？」
- 「実装が明解だと思いますか？」
- 設計意図について妥当だと思いますか？」
「ここの実装に納得いってない。こうした方がスマートなのでは？と思ったりする」みたいなコメントをした後にセルフツッコミで自己完結して直すパターンもよくある
- セルフツッコミを制するものはセルフレビューを制する

PRはもっと小さく分けられないか確認する

ここまでコメントしていく中で不必要なものとか、分かりづらいものが見えてくるので、一気に判断してしまいましょう。

無駄なコメントアウトしたコードだったり、デバックコードなどを消し忘れていないか確認する
- 不必要なものはCIで弾いてしまう仕組みを作るのも吉
PRが大きいとレビューが大変になってしまうので、小さく切り分ける
- 案外、想像以上に小さくできるし、した方がいい (場合によるが)
- 個人的には100行を超えると「本当にこれって別々のことを1つのPRでやってないよね？」を自問自答する

PRのタイトルやDescriptionがレビュアーに過不足なく情報を伝えられているか確認する

ここでは思いっきり初見レビューの気持ちで見るのが重要です。

タイトルで何をやったのか(HowではなくWhat)を伝えられているか
- よくあるのは「class A ではなく class B を使うようにした」のみが伝えられ意図がない場合は「それは何が嬉しいの？」となってしまう
- 例えば可読性が悪いとか、テストを書きやすくするためだとか、機能を満たせていなかっただとか、何をするためのPRなのか伝える
PR Descriptionにはタスクの背景(why・なんでやるのか？)、画面が変わったならbefore/afterのスクショ、レビューして欲しい観点、懸念点などを記載されているか
- 細かいPR Descriptionの書き方tipsみたいなのは別で書きたい気持ち

まとめ

「よっしゃPRできた！」「(他の人からの指摘と質問の嵐)」「ぐあああああああ！！」のパターンを何度も踏んできました。慣れないうちはセルフレビューは時間がかかるなーと思ってしまうかもしれません。

ですが、最初の単純なミスや見落としが後に影響が出てきて手戻りして辛かったり、意図はあったけどレビュアーに伝えてなくて質問のやり取りに時間を使ってしまったり、トータルで見ると案外レビュー依頼してからのやり取りに多く時間を使っているものです。
もしくはそもそもレビューをあまりしていなくて無秩序な状態の可能性もあります。

私の個人的な感想では、セルフレビューをするだけで多くの問題は自己完結できるし、レビューの速度も倍ぐらいにはなっているんじゃないかと感じています。
セルフレビューをやってみてはいかがでしょう。