#6で「よりよいレビュー環境を求めて」という発表をしたので資料を公開します。以下、発表内容を文字に書き起こしたのでよろしければお読みください。Requestの作り方を工夫してレビュアーの負担を減らそう細かな改善を重ねてレビュアーが疲弊しないような環境作りをしていこうDisclaimer本稿は、はてなブログを開発しているチームが日々取り組んでいることを紹介するものです。まるっと他のプロダクトの開発に転用することは難しかったり、必ずしも効果を上げられるものではないでしょう。この内容がみなさんのチームのレビューにまつわる諸々を見つめ直すきっかけになれば幸いです。理由は様々ありそうですが、ソフトウェアの質を担保するため、という点を欠かす人はいないと思います。レビューの質が下がれば必然的にソフトウェアの質が下がります。これを回避するために私たちレビュイーができることは、レビュアーをもてなすことです。様々な手段を駆使してレビュアーの負担を軽減させることがソフトウェアの質につながるのです。ひとつ注意したいのは、レビュアーが優位だとか、レビュイーが格下だとか、そういうことを言いたい訳ではないということです。レビューとはすなわち対話であり、ソフトウェアの質を良くするための手段でしかありません。ソフトウェアの質はチームで協力しあって保持するものです。そのための努力を惜しんではいけないというのが本稿の本質です。本題に入る前に、はてなブログの開発フローについて紹介します。これから紹介する内容の前提です。現在のはてなブログは、エンジニアが6名、デザイナが2名で開発を行っています。実際にはさらに細分化され、C向け(はてなブログ)チームとB向け(はてなブログMedia)チームにそれぞれ半数ずつ所属しています。はてなブログとはてなブログMediaは、Perlベースのモノリシックなアプリケーションで、チームに関わらず1つのリポジトリのみで開発を行っています*1。新機能を作る際は、masterブランチからトピックブランチを切って変更を加え、レビューを経てmasterブランチにmergeします。masterブランチにある程度変更がマージされたらreleaseブランチにマージしてリリースします。1日2回、14:30と17:30にレビュータイムがあり、基本的にはレビュー待ちのタスクをゼロにすることになっています。リリースは1日0回〜2回程度で、ほぼ毎日リリースを行っています。レビュータイムでは、エンジニアによるエンジニアレビューとデザイナによるデザインレビューが行われます。Requestを作りますし、レビューも行います。Requestのパターンと、それを回避するための戦略さて、ここからが本題です。個人的に、またはチームとして、コードレビューの負荷軽減のために行っている取り組みをいくつかご紹介します。Requestのパターンと、それを回避する戦略のの話をします。Request単位で行うものですから、この話を避けては通れないでしょう。今回紹介するパターンは以下の4つです。一発入魂パターン大会場パターン数珠繋ぎパターンバザールパターン時と場合によってはこれらのパターンを採らざるを得ないこともあります。そのため、バッドパターンではなく避けることが望ましいものとして提示しています。このパターン名は自分が勝手に考案したもので、特にチームで共通の語というわけではありません。一発入魂パターン一発入魂パターンは、新しい機能をすべて1つのトピックブランチで作りきるパターンです。一発入魂パターンごく小さな変更であればさほど困ることはないかもしれませんが、変更の行数が肥大化するにつれて以下のような問題が発生します。Requestを作るパターンです。Request自体は細かくなっておりレビューがしやすく保たれているため一見良いように見えますが、以下のような問題があります。一発入魂パターンと同じく、新機能リリース用ブランチをmasterにマージする際にコンフリクトしやすくなる機能全体はレビューされているが、それをmasterにマージしたときに動くことの保証が薄い統合のレビューがないため、masterが大会場ブランチを作成した時点から乖離すればするほどこの傾向が顕著になるレビューを受けないと先に進められないタスクでブロックされることがある数珠繋ぎパターン数珠繋ぎパターンは、大会場パターンの亜種です。レビュー待ち状態のトピックブランチから新たにブランチを切って、作業を先行するパターンです。Requestが2,3つ程度で完成する中小規模な改修で時々目にすることがあります。Requestを細かく保ちつつもレビュー完了に先立って実装を進めることができるため大会場パターンで発生する作業のブロックを回避するという点です。基本的には大会場パターンの亜種のため同様の問題があり、加えて以下のような問題も発生します。Requestをmasterに向けてマージするか、どちらかをとる必要があるバザールパターンバザールパターンは、1つのブランチで複数人が作業するパターンです。バザールパターン通常、複数のエンジニアが1つのブランチで作業することは極稀ですが、エンジニアとデザイナが同じブランチで作業することがあります。この場合、以下のような問題が発生します。Requestのパターンを紹介しました。Requestとはどういうものでしょうか。僕は以下の2点を満たすものがそれだと考えています。レビューすべきコードの量が少ないレビュアーの負うべき責務がはっきりしているレビューすべきコードの量が少ないというのは当然その通りでしょう。見るコードの量が少ないほど変更の影響範囲も狭くなるため考慮すべき事項もそれに応じて減少し、レビュアーの負担が軽減されます。Requestにその趣旨に沿ったコードのみが含まれている状態のことです。Requestをレビューしようとすると、どのコードがレビューの対象でありどのコードがそうでないかを意識的に選びとる必要があります。この作業は意外とレビュアーの負担になりうるものです。Requestとは関係ない便利ツールが追加されていたり、別のブランチからのcherry-pickが多かったりするのもレビュアーの負うべき責務がはっきりしていない状態といえます。これについて、僕は以下の点について留意すると良いと考えています。Requestは細かくmasterにマージする実装にあたって必要なタスクを事前に細かく分割してまとめておくまず重要なのは事前に必要なタスクを洗い出してまとめておくということです。当たり前なことかもしれませんが、慣れてくると中小規模の機能では意外とスキップしがちなことがあります。これを行うことで、以下のような効果を得ることができます。Requestがプロジェクト中のどのフェーズのものかをレビュアーに視覚的に提示できるさらに、ここで留意すべきは以下の2点です。分割したタスクをすぐさまIssueに落とし込まない作業時間がなるべく一定になるように分割する1つのタスクで作業する開発者はなるべく1人に留める分割を終えたタスクは、実際に作業してみると共通化できる部分があったり、またはコード量が増えてしまうので一旦区切りをつけたくなったりと、更に分割したり結合したりをしたくなるタイミングがままあります。そのためすぐさまIssueに落とし込むのではなく、ひとつのまとめIssueを作ってタスクの進行度合を示すチェックリストを書いておくと効果的です。GitHubでProjectsを利用している場合はレーンにカードを追加することができるので、これを利用しても良いでしょう。することでIssueを破棄する手間がなく自由にタスクを分割したり結合したりすることができます。Requestが肥大化することがあります。このような心理的障壁を取り除くのは意外と効果があるものです。作業時間がなるべく一定になるよう心がけて分割するのも効果があります。Requestのレビューを行う時間がチームで固定されているときに効果的です。レビュータイムを終えてから新たなタスクに着手し、次のレビュータイムまでに終えられる程度の粒度に分割しておくことで、ブロッカーとなるタスクの影響度を最小限に抑えることができます。この点については見積もりの精度の高さが要求されますし、正確に見積もるためにはある程度の経験と慣れが必要になるため、あまり拘りすぎないことも大切です。1つのタスクで作業する開発者はなるべく1人に留めるという点も留意して分割できると良いでしょう。Requestで行ってしまうバザールパターンに陥りがちです。タスク分割時点でこの点にも留意すると、レビュアーの負担を軽減することができます。Reequestを細かくする、つまりレビューすべきコードの量をへらすための留意点でした。Requestを細かくmasterにマージする手法は、レビュアーが負うべき責務をはっきりとさせるためのものです。masterがトピックブランチを作成した時点から乖離すればするほど、トピックブランチをmasterにマージした際に正常に動く保証がなくなります。トピックブランチで作成した機能をmasterにマージする前に別のトピックブランチでも使いたい場合そのコミットをcherry-pickをする必要があり、それらのコードをレビューする責任の所在が不明瞭になります。Requestを細かくmasterにマージする手法が有効です。基本戦略は簡単で、ユーザ影響の無い部分から作り、順次masterにマージしていくという手法をとります。ブログサービスで記事にコメントをつけられる機能を作る場合、以下のようにタスク分割を行い順次masterにマージしていくことで機能を完成させることができそうです。Blogにて解説されていますので、こちらも参考にしてみてください。developer.hatenastaff.comレビューを取り巻く環境を工夫するここからはレビューに関連するいくつかの改善の取り組みについてご紹介します。Requestをゼロにする活動を行なっています。ベストエフォートであるため特定のエンジニアにレビュアーが偏ることがままありました。レビュアーに偏りがあるということはそのエンジニアが持っているタスクの進みがレビューで律速されるということであり、チームとしては不健全な状態といえます。id:aerealさんがレビュアーの偏りを可視化するツールを作成し、チームに導入しました。Requestのレビュアーを示すグラフです。Requestが2人のエンジニアによって行われているということがわかりました。レビュアー率の高くないエンジニアはC向けチームのエンジニアに寄っており、レビュータイムのリマインダがC向けチームの日例と近いことが原因という予想が立てられ、リマインダの発火時間をずらすことになりました。この後リマインダの時間変更を含めいくつかの施策を行った結果、実際に偏りをある程度均一化することができました。現在のグラフは以下のような状態になっています。6月のレビュアーの偏り具合これでもまだ偏りが存在しているため、更に均一化を進めるのが今後の課題ということになりそうです。このグラフだけでエンジニアのチームへの貢献度を測ることはできません。Requestの粒度が全く同一に揃っているわけではありませんし、レビュアーのコンテキスト理解の有無などによってもレビューにかかる時間や負担の具合などが変わってくるため、このグラフをエンジニアの評価に直接転用することはできないという点はチームでも共通認識として持っています。レビュアーの偏りの傾向などはこのグラフから十分に読み取れるので、それを均一化するための対策を立てるための情報源として非常に有益といえそうです。アルバイトスタッフにもレビューをしてもらう冒頭でレビューはソフトウェアの質を担保するために行うと述べましたが、その他にも以下のような効能があると考えています。Requestの粒度、分割の手法等を学ぶことができるもともとアルバイトスタッフはレビューをしない運用でしたが、この考えをもとに最近はアルバイトスタッフにもレビューを行なってもらっています。プロパーのエンジニアと同様にレビューするのではなく、以下のような制限を設けてこの範囲で自由にレビューしてもらうことにしています。レビュータイムは1時間に制限するアルバイトスタッフがapproveしてもmasterにマージはできない時間制限については、単純にレビューに時間をとりすぎることを防ぐために設けています。プロパー社員よりも就業時間が短いためその多くをレビューに割いてしまうと本来お願いしたかった業務に滞りがでてしまうためです。アルバイトスタッフがapproveしてもマージできないのは、コードをリリースする責任を負わせないための仕組みです。レビューを行うということは、そのコードをリリースする責任を実装者とともに負うということでもあります。責任をアルバイトスタッフが負う必要がないようにこの制限を設けています。これは単に運用上のルールというだけでなく仕組み化もされています。Requestを行うフローになっています。Requestはapproveされない限りmasterにマージできないようにプロテクトを掛けています。することで、このグループに参加していないエンジニアがapproveしてもmasterへのマージはできないような仕組みを作ることができます。チームに新たなプロパー社員が参加したてのときも同様の方法で、approveは可能だがマージはブロックする、という運用を行うことができます。これを実践してみて実際に効果が上がっているかについては、取り組みを始めてから1ヶ月程度ということもあり正直なところまだわかっていません。現在はこの取り組みと平行して、大きなタスクに着手する際は事前にタスク分割のレビューも行う、といった取り組みも行っています。Requestの粒度については心がけてもらえている状態です。まとめ以上、レビュアーの負担を減らす取り組みについてご紹介しました。レビューはソフトウェアの質を担保する重要なファクターですが、意外とレビュアーの負担というものは軽視されがちなものです。細かな改善を重ねてこの負担を軽減することで、ソフトウェアの質の向上にも貢献できるのではないかと思います。これを機に本稿をお読みのみなさんもレビュアーが疲弊しないような環境づくりに取り組んでみてはいかがでしょうか。読者になる関連記事2022-09-02ロールに求められることと名前は一致させたいいま所属しているチームには相談役というロールがあって、タス...2022-08-26CDKでEC2を建立するときになんかするめっちゃ小ネタだし知ってるわって人も多そうな話題なのだけど...2020-10-09別ブランチの特定のファイルを直接開くコマンド欲しいときがありますよね。v2.1.0が出て問題は解消されました。10はてなブログをはじめよう!masawadaさんは、はてなブログを使っています。ブログを報告する引用をストックしましたストック一覧を見る閉じる引用するにはまずログインしてくださいログイン閉じる引用をストックできませんでした。再度お試しください閉じる限定公開記事のため引用できません。