Developer Enablement Monthと、そこから生まれたコードレビュー手法
こんにちは、よしこです。
みなさんコードレビューしてますか？
今日の記事では、最近おこなわれた社内でのイネーブルメントを推進する取り組みと、そこから生まれた新たなコードレビューのやり方についてご紹介しようと思います。
コードレビューにおけるトレードオフ
取り組みやレビュー手法の話をする前に、前提としてコードレビューの際に以前から私が持っていた悩みをお話しします。
（以降レビューする人をレビュアー、される人をレビュイーと記載します）
ナレッジワークのフロントエンドチームでは私が主なレビュアーとしてコードレビューをしているのですが、その際に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 なアプローチのページが前述のトレードオフを解決するヒントになりました。
公開されている資料で引用すると以下のページです。
弊社では「コードを書く前に設計をする」という文化は既に根付いており、会社説明スライドの中でも27ページ目で「DesignDoc」として紹介をしているのですが、こちらはドキュメントベースの設計書となっており、設計の中でもレイヤーが高めの内容でした。
抽象的で後戻りしづらいモデル定義やデータの流れ方などを設計し擦り合わせる際にはとても威力を発揮していましたが、フロントエンドの特にView部分のような「大きなUIをどのような構造で実現するか？」というかなり具体に迫った設計には活かしきれておらず、実装する中で考えるスタイルになっていました。
対して、上記に引用させていただいたTop-down approachのスライドはコードレベルでの設計となっています。この、ちょうどドキュメントとコードの間のような設計スタイルが今のフロントエンドのコードレビューの課題にうまくハマるのではないかとヒントを得ました。
スケルトンレビューの誕生
Developer Enablement Monthの講義のあと、さっそくこのヒントを形にしてみようと思いました。
経験上、コードレビューの段階でレビューによる手戻りが入ったときに変更量が大きくなるのはViewのレイヤーでした。
まず1つのComponentを構成するためのファイル数がtsx, css, storyなど多いですし、Componentの分割単位やインターフェイスが適切でないというレイヤーが入った場合、内部を構成するComponentがほとんど書き直しになるケースもあります。
逆にView以外の裏側のレイヤーは、ModelやGlobalStateであればデータ構造として前述のDesignDocでも設計ができますし、UsecaseやRepositoryなどのレイヤーは既にアプリケーション全体で枠組みを整えてあるので、ゼロから設計しなければならないことは少ないです。（このあたりはまた別の記事でご紹介できたらと思っています）
よって、今回考える手法でのレビュー対象は以下に定めました。
（新しく作る/既存に手を入れる）Component名・役割・インターフェイス（Props）
（新しく作る/既存に手を入れる）Component同士の関係性の全体像
今まで実装しながら考え、コードレビューの段階ではじめてレビュアーに見せていた上記のポイントを事前にすり合わせることができれば、大きな手戻りの減少を期待できます。
かつ、これらはかなり具体に近い部分の設計なので、ドキュメント上ではなく生きているリポジトリのコード上で設計を示すことができれば、同時に実装の第一歩もまかなえると考えました。
これをフローとして策定し周知したのが以下のスライドです。
そして策定の一環として、まず自分で実践してみました。上記スライドの「参考PR（Pull Request）」というリンクがそれで、以下に画像として紹介します。
descriptionはこのように、図でComponentの種類・名前・インターフェイス・関係性を明示しています。
（最初の実践だったので図もPull Requestと一緒に出していますが、本来は図ができた段階で先にSlackで擦り合わせることを推奨しています）
コードのほうでは新設する各Componentを社内の標準テンプレートでScaffoldしたあと、Propsの型定義と、内部が今後どう実装されるのかのコメントだけをしています。（Scaffoldについては後述）
以下の画像はPull Requestに含まれるひとつのtsxファイルの例です。
このように先に構造だけを設計してマージするフローをとることで、レビュアーは以下のようにレビューが楽になります。
全体像を把握した上で各部品のインターフェイスや責務についてのレビューができる
細部の実装ロジックが含まれないため、コード量が少なく、構造のレビューだけに集中できる
このスケルトンのPull Requestをマージしたあとは各TODOコメントに対してそれぞれに実装Pull Requestを貰えばいいので、後続のPull Requestも差分を小さくできる
レビュイーにも以下のメリットがあります。
まず構造だけ考えてレビューを通すことで、フィードバックサイクルが小さく早くなる
分割単位や構造を考え直すことになっても、実作業はScaffoldコマンドを叩き直して必要に応じてコメントをつけていくだけなので、手を動かし直す分量が少ない
詳細実装まで書ききってから根本的な指摘を受けて全て手戻る、というような事態を防げる
構造と詳細ロジックへのレビューが同時にくることがなく、ひとつの思考に集中できる
上記の参考Pull Requestと共にフロントエンドチームに提案してみたところ、とても好評を得てすぐに他のメンバーも実践してくれるようになりました。
すべての開発に適用しているわけではありませんが、大きめの新規機能開発では特に威力を発揮してくれています。また、それを見ていたバックエンドのメンバーも自発的にバックエンドの開発にも取り入れてみて、良かったというFeedbackをくれたりしました。
ナレッジワークにおける開発のひとつの武器になりつつあるかなという実感があります。
余談： ScaffoldとSPRINT_TODO
上記の説明の中でScaffoldとSPRINT_TODOという言葉が出てきましたが、これらもナレッジワークの開発を支える工夫のひとつなので、あわせて簡単にご紹介できればと思います。
Scaffoldはフロントエンドのリポジトリに用意している仕組みで、用意した雛形から基本的なコードを生成できる仕組みです。 Component、 Page、 Modelそれぞれに雛形を用意しており、それらを新設する際にはまずコマンドを叩いてベースのコードを生成し、それを編集していくフローになっています。アプリケーションを通して一貫した構造を保つのに不可欠ですし、生成と編集でコミットを分割することで、前述したスケルトンを書くほどではないサイズの新規Componentのコードレビューの際にも独自のロジックの箇所が明確になり、重宝します。
Scaffoldツールとして scaffdog を愛用しています。Markdownベースなので、雛形ファイルがドキュメントのようにもなる特長があります。
各雛形の内容や詳しい運用についてはまた別の記事でご紹介できればと思います。
SPRINT_TODOというのは、「あとでやろうと思ってたけど忘れてた！」を防止するための仕組みです。
リポジトリのコード中にSPRINT_TODOという文字を含めたコメントをすると、その行の存在が毎朝Slackに通知されるようになっています。
ナレッジワークでは2週間のスプリント開発をしているので、「今すぐには別の待ちがあって出来ないんだけどスプリント締日までには必ず実装する必要のあるTODO」がたびたび登場します。ただのTODOコメントだと書いた人がきちんと覚えておかないと対応し忘れてしまいますが、SPRINT_TODOと記載しておけば毎朝通知がくるので頭のリソースを開放することができます。スプリントの締日が近づいたら、通知の中にそのスプリント対象のSPRINT_TODOが残っていないか？をチェックします。
これは以前にエンジニアチーム内で「あとでやろうと思ってたけど忘れてた！」が発生したことがあり、振り返りでその対策として出てきた案で、通知の仕組みは業務委託でお手伝いいただいている horiさん がシュッと作ってくださいました！作成当日から活用しまくっています！
結び
この記事ではナレッジワークにおける「新しい学びを推進する取り組み」であるDeveloper Enablement Monthと「得た学びを業務に転換した実例」であるスケルトンレビューについてご紹介しました。
最後に、貴重なお時間を割いてエンジニアチームに学びの機会を提供してくださった石川さんに改めて感謝いたします。
Developer Enablement Monthで講義いただいた内容については、英語版が Code readability としてスライド公開、日本語版が 読みやすいコードのガイドライン という書籍としてちょうど先月刊行されています！ とてもおすすめできる内容ですので是非。
ナレッジワークではこれからも様々な学びを得られる機会を作っていき、日々の業務を進化させていこうと思っています！
ご興味を持ってくださった方はぜひ 求人情報 または以下のカジュアル面談フォームよりご応募ください！