Pull Requestレビューの負荷を下げるためセルフチェックを導入してみた

こんにちは。
ユニバーサルゲーム事業部でサーバーエンジニアをしている鈴木(高)です。

今回は、プログラム修正後のレビューについて、作業負荷を下げるためにセルフチェックリストを導入したことについての記事となります。

……チェックリストと聞いて、面倒そうな印象を抱いた方もいるのではないでしょうか。わかります。

実際、Pull Request(以下PR)を提出する前の手間はすこし増えます。自分もやってみて面倒に感じることは否めません。

ただその点を差し置いても、メリットとなる効果はあったかなと感じています。

導入のきっかけ

私のいるチームではプログラムを修正した後で、GitHubのPRを作成してチームメンバーにレビューをしてもらっています。

自分がプログラムを修正してPRをレビューしてもらうことも、他の人が作成したPRをレビューすることもあります。

そして毎日そういったPRをレビューしていたところ、別々のメンバーから提出されているPRでも、似たような指摘内容があることに気づきました。

またその内容はチーム内で共有しているコーディングのガイドラインに照らし合わせれば客観的に判断できるのではと、そう感じることもありました。

そこで…提出前にセルフチェックをしてもらえたら、よくある指摘やフィードバックを減らすことができないかと考えて、具体的にリストを作成することにしました。提出前に指摘の元を減らすことで、レビューする人とされる人の両方が楽にならないかと。

導入した結果は

そしてざっくり1〜2時間程度でチェック項目をリストアップしてスプレッドシートにまとめ、サーバーエンジニア内で共有を行い、そのリストを使ってPR提出前にセルフチェックをお願いしました。

メンバー全員に実施してもらい、導入日前後の3ヶ月とおよび直近3ヶ月における「チェックシートでカバーできそうなフィードバックコメントの数」をカウントした結果がこちらになります。
(私が「チェックリストの内容に当てはまるかな」と独自に判断したコメントをカウントしました)

直近3ヶ月の効果がすごいですね…

ここ最近そういった指摘をすることが体感的にも減っていると感じていたので、その面でも納得の結果です。

(そして3ヶ月で71件、PRが出ていたことに驚いています…もちろんPRごとに修正量の多い少ないはありますが…これだけのペースでコードに修正が入っているのですね…)

ただ逆に、導入直後すぐに下がることはなかったようです。導入後と直近3ヶ月では6月〜8月あたりの時期が被っていますから、結果を分けたのは最初と最後の1ヶ月になります。

そのときに行っている修正内容や忙しさによっても左右されますし、また始めたばかりだったことも影響しているのかなと考えています。

またチェックリストに掲載していた項目はそのままレビューで引っかかるポイントでしたので、フィードバックに慣れてきた結果、あらかじめ対応してもらえるようになった面もあるでしょう。

こういったことも踏まえてチェックリストだけの効果ではないと考えてはいます。ただ導入してから数値が改善されてもおり、 プラスの効果はあったかなと感じています。

現在はセルフチェックの実施は任意となっています。上記のように状況が改善され、体感的にも負担が減ったので必須ではなくて大丈夫となったためです。

導入してよかった効果

さて、導入してみて個人的に感じた効果についても記載します。

デメリットについては 実施する手間が発生する ことくらいですから…もちろんこれはこれで大きいです…この手間をかける価値があるかですね。

レビューするポイントが明確化された

リスト形式でチェックポイントを提示することで、見るべき内容がわかりやすくなったと感じています。

もともとコーディングなどに関するガイドラインは作っていましたが、全てを把握することも大変だったため、ひとつずつ確認できる形式にまとめてよかったです。

自分の見直しにも使える

自分で作っておいてなんですが…セルフチェックをしてみると、やはり 自分でも修正点が見つかること、よくあります。

提出して、誰かに指摘をもらってからだと二度手間になってしまいますし、見ておいて良かったなと感じるときです。

本質的なレビューに集中しやすくなった/修正コストが減った

典型的な指摘点が減ったことで、最初から本質的なレビューへ集中できるようになったと感じています。

コメントが増えていくとPR全体が見辛くなってしまうので、コメント量は少ない方が良いですから。

また同時にレビューを受ける側にとっても、細かい修正で差し戻される回数が減ることは、負荷軽減に繋がると考えています。

セルフチェックリストの項目で注意していること

セルフチェックリストへ掲載する項目についても、一定の注意を払っています。

ただ注意点を詰め込んでいくだけでは、かえって形骸化しまうと感じています。

また仕様や動作の確認などによる、PRそれぞれの状況にあわせた指摘コメントは必要だと考えています。

そのためチェックリストによってレビューでのフィードバックコメントをゼロにすることは目指していません。

客観的に判断できる項目を選ぶ

まず気をつけているのは、誰が見ても異論なく判断できるもの、ガイドラインとして共通認識になっているものを対象にすることです。

人によって判断が分かれる項目は基本的に記載せず、個別にフィードバックすることを想定しています。

たとえば、チームで設定しているガイドラインでは、「ファイルの最後に改行をいれること」にしています。

これはGitHubのレビューでファイルを見ると 最後に改行が入っていないファイルにはそれを示すマークが表示される ため、客観的に判断できます。

根拠や理由も記載する

ただチェック内容を書くだけではなく、どうしてそれをNGとするのか、注意する根拠や理由も掲載します。

私が項目を出したとき、もともとガイドラインとして決めていたことはそれを記載しています。

そうではないけれどレビュアーの観点からチェックしてほしいことについては、その理由を記載しました。そしてそれを全体に共有しています。

これは後々「どうしてこれをチェックしているんだろう…」と思われたり、謎ルールとならないようにするためです。

また、上で「客観的に判断できること」と書きましたが…主観が混ざる項目はゼロとは言えません。そういった場合に、判断の参考となることも期待しています。

あくまで「典型的なフィードバックの発生率を下げる」ためのものと捉える

作ったリストを実施してもらう時の思いとして、「典型的なフィードバックの発生率が下がればよし」と考えています。

やはりセルフチェックですし、チェックしたつもりでもすり抜けることはあると思っています。最初に記載した結果でも、減りはしましたがゼロにはなっていません。リストがあるからと完璧なチェックを目指したら、それはそれで疲れてしまって続かないかなと思ってこうしています。

この方向性はリストと一緒に共有しています。

さいごに：やってみた感想として

この方法が完璧でないことは承知した上で、叩き台を1〜2時間で作って試した施策としては良い効果だったかなと思っています。

正直、自分がセルフチェックする時には面倒さを感じますけれど…そのチェック中に気づいた点がある場合など特に、レビューに提出してフィードバックを受けるよりはマシだったかなと感じます。

実施する手間などもあるため、どの環境でも良い効果となるかはわかりません。ただレビューの負荷を下げたいときの一例として、参考になれば幸いです。