クックパッド開発者ブログスマートフォン用の表示で見るクックパッド開発者ブログ2015-03-30コードレビューに費やす時間を短くするはじめにこんにちは、広告事業部の芳賀(@func09)です。普段はクックパッドの広告配信周りや純広告・タイアップ広告などの商品開発を行っています。レポート、メール配信、などなど様々な担当者がいます)と業務をすることが多くてコミュニケーションが上手くいかず業務がスムーズに進まないことがありました。当たり前のことではありますが、エンジニアにしかわからない言葉は使わないとか、できるだけ相手の業務を理解し相手の考え方や視点に立って話すなど、ちょっと工夫することで、長引きがちなMTGや相談がすんなり終わったり、お互い良い気分で終わることが多くなって、費用対効果が高いなと感じています。一方でエンジニア同士のコミュニケーションでも時間がかかってコストが高いと感じることがあります。それはプルリクエストを使ったコードレビューでのコミュニケーションです。今回はコードレビューにかかる時間的コストをさげて、生産性を上げてこ!という話をしてみようと思います。コードレビューの良さと悪さ弊社では自分の書いたコードをいきなり本番環境にマージするなんてことはなく、まずプルリクエストを送ってチームメンバーからコードレビューを受けたり議論したり、コラボレーションしながら開発を進めています。コードレビューにはプロダクトの品質向上に繋がるという素晴らしい良さがある一方で、コードレビューにかける時間やプルリクエストが送られてくるタイミングは予測しづらく、結果的に自分の作業にかけられる時間が減ったり、頻繁なコンテキストスイッチが発生して集中力が下がってしまうデメリットを感じています。それらはコードレビューから受ける恩恵と比べたら、大したデメリットではないとも思うのですが、できるだけ自分の生産性は下げたくないし、同様に他のエンジニアの生産性も下げずに済むにはどうすればいいか?そんな事を考えていました。コードレビューを開始するまでの時間を短縮する私にとってコードレビューに時間のかかるプルリクエストは規模が大きく、差分が多い知らない機能、文脈のわからない変更についてのプルリクエストコードを読まないと変更点の意図が理解できないというような特徴を持っていました。「規模が大きい」という点は、プロジェクトによっては差分を小さくできないこともあるので仕方ないと思います。他2点は、コードを読む前にレビュアーに効率良くインプット可能で、回避可能なことなので、そこは工夫していこうと思っています。そこで私は「レビュアーがコードレビューを開始するまでの時間を最短にする」という事を目標に、プルリクエストを送るようにしています。レビュアーがコードレビューを開始するまでの時間を最短にするにはコードレビューではマージする価値があることがわかるリスクについて十分考慮されている(明らかなバグが見つからない)ことがわかるコードのメンテナンスが可能であることがわかるという点のいくつか、またはすべてをレビュアーは見ていると思います。文脈を良く理解している、もしくは熟練のレビュアーならコードを読むだけでもチェック可能かと思いますが、コードを読む負担を可能な限り減らすために以下のような工夫をしています。伝える内容を選択する無駄な情報は捨てる注意を引く規模にもよりますが、説明文を読んで1分くらいでコードレビューに入れる状態が望ましいと考えています。伝える内容を選択するたくさん書きたくなってしまったり、逆にめんどくさくてちょっとしか書かなかったり、そういうブレがないように伝える項目を決めておきます。私は次の2つの項目を必ず入れるようにしています。目的達成のためにやったこと(手段)「目的」はできるだけひと言で表現し、「手段」は箇条書きで表現します。目的は、そのプルリクエストをマージするとどういった価値があるのかシンプルに表現します。手段は、目的を達成するためにどういう実装をしたのか表現します。それらは「何をやるの?」と詳細に降りていったり、逆に「なぜ必要なの?」と遡れる関係にあるとレビュアーの理解の助けになると思います。無駄な情報は捨てる説明文を読むのに時間がかかりすぎるのは、逆に混乱を招くので必要な部分だけにそぎ落としておきます。チームメンバーにメンションを送る前に、もう一度読み返してみて、長文になっていたり、論点からはずれている説明があれば削除するか、ページの下の方に補足として読みたい人だけ読んでね、というスタンスに組み立てなおします。その他の内容はケースバイケースで選びます、レビュアーに他部署の人がいれば、文脈共有のために「経緯」を書いたり、UIの変更点が重要な場合は画面キャプチャを貼ると理解が早いので積極的に利用しています。下記はこれまでの内容を元に架空のプルリクエストの説明文を作文してみました。私が送るプルリクエストの説明はだいたいいつもこれくらいの文量です。単純な例ですが、これを見た人がどんなファイルにどんな変更点が加わるのか、Diffを見なくてもなんとなくわかる状態が作れていれば、「コードレビューを開始するまでの時間の短縮」に繋がって、レビュアーの時間が少しでも多く確保できるのではないかと思っています。注意を引く伝える内容を選択して無駄を捨てた上で、レビュアーの理解を助けるために注目すべき点を明瞭にします。レビュアーをサポートしたいならどこから読み始めると理解しやすいのかコメントする読む必要がない箇所を教えるレビュアーに安心感を与えたいならテストがきちんと通っていることを伝えるマージ後の動作確認フローを伝えるレビュアーにヘルプを求めたいなら実装で迷っている点を伝えるリスクがありそうな箇所を伝えるなどです。すべてを毎回いれるというよりは、必要に応じていれることがあります。相手の思考を先読みして、相手にどういう行動をとって欲しいのか注意を引くことでコントロールします。本記事ではほとんど触れていませんが、コードそのものが読みやすいことが何より1番大事なのは言うまでもありません。エンジニアはリーダブルコードのような、よりシンプルに明瞭にするという考え方が根付いているので、あえて書く必要がないようなことだったかもしれません。書きましたが最終的には「変更点が相手に伝わっている」状態が作れていれば、どんなプルリクエストでも良いと思います。この記事が皆さんや周囲のエンジニアの生産性向上の助けとなり、より良いアウトプットに繋がれば幸いです。会員事業部の三吉です。使ったセキュリティログ検索基盤の構築サービス特性にあった検索システムの設計戦略クックパッド採用説明会「クックパッドはサービスの作り手を採用したいんです。/*}*/スターをつけました引用をストックしましたストック一覧を見る閉じる引用するにはまずログインしてくださいログイン閉じる引用をストックできませんでした。再度お試しください閉じる限定公開記事のため引用できません。