https://note.com/knowledgework/n/n50fc54509dd5

こんにちは、よしこです。 みなさんコードレビューしてますか? 今日の記事では、最近おこなわれた社内でのイネーブルメントを推進する取り組みと、そこから生まれた新たなコードレビューのやり方についてご紹介しようと思います。

コードレビューにおけるトレードオフ

取り組みやレビュー手法の話をする前に、前提としてコードレビューの際に以前から私が持っていた悩みをお話しします。 (以降レビューする人をレビュアー、される人をレビュイーと記載します)

ナレッジワークのフロントエンドチームでは私が主なレビュアーとしてコードレビューをしているのですが、その際に1つ悩ましいトレードオフがありました。

これはレビュアーにもよるかもしれないのですが、私の場合は「全体像を見てレビューしたい」という方針があります。 「ある振る舞いを実現するコードのうち一部だけのコード」よりも「ある振る舞いを実現するコード」をまとめてレビューしたい。 具体的な例で言うと、「まだどこからも呼ばれていない関数Aを追加するコード」だけだとレビューがしづらいので、「その関数Aを使う側のコード」まで含めてPull Requestにしてもらってレビューしたい、というような感じです。 部品1つとっても、頭の中での想定だけだと実際に使う段階ではじめて見落としていた要件が出てくる場合もありますし、利用箇所や文脈次第でより良い形が見えてくる可能性もあるので、なるべく最終的にどうなるのかを把握できる単位でレビューしたいなという気持ちがありました。

しかし、それを実現しようとすると必然的にPull Requestのサイズが大きくなるというトレードオフがあります。 Pull Requestが大きくなると

など大変なことも多いですよね。

一方で、Pull Requestを小さくすると、全体像が見えないまま個々のレビューをすることになるため

などの点にあとから気付く、ということもありえます。

このトレードオフには長らく頭を悩ませていました。 2つを比較したとき、自分にとっては前者のほうが「自分が分量の多いレビューをやりきれれば後者のデメリットをとらなくて済む」という点でまだ良いと思えたので、なるべく素早いレビュー速度を心がけつつ前者のスタイルをとってきました。 しかし、Pull Requestが大きいということは付けるレビューコメントの数も多くなりがちなのでレビュイーにもプレッシャーなわけで、これから新しいメンバーが増えていくにあたって何か解決案があるとよいなと思っていました。

Developer Enablement Month

そんなとき、CTOのmayahさんのご友人であるLINE株式会社の石川宗寿さんが、ご厚意でナレッジワークのエンジニアチームに「読みやすいコードの書き方」についての講義をしてくださることになりました。 この会には、イネーブルメント(できるようになること)を推進するナレッジワークらしく「Developer Enablement Month」という名前がつけられ、エンジニアチームは毎回とても楽しみにしていました。

Developer Enablement Monthでは隔週開催の講義で「プログラミング原則」「命名」「依存関係」などコードライティングに広く活用できる計8つのテーマを扱い、2時間x4回にわたる充実ぶりだったのですが、その講義の最後のトピックがまさしく「コードレビューについて」でした。 大きすぎるPull Requestは避けようという文脈の中で、それを実現できる2つのアプローチが紹介されており、その中のTop-down なアプローチのページが前述のトレードオフを解決するヒントになりました。 公開されている資料で引用すると以下のページです。