🙆‍♀️

いきなり1,000行越えの差分のあるPRのレビューを依頼するのは今すぐやめろ。何年前の開発スタイルだよ

2022/08/10に公開約5,300字1件のコメント

今回語りたいこと

PR/リリース単位の違いの変化を時代と共に語るというのが主旨です

この記事を書いた意図

最近開発速度を上げたい的なお話が多く、その際に何回も同じ話を説明することになっているので改めてここに書いておこうと思います。
もう少し膨らまして講義としてまとめた上で、オンボーディング研修の受講動画にしたいですね。

変化をざっくり言うと

リリースされる単位は時代が下るごとに細かいサイズでリリースする方向に変化しています。
近年だと平均差分の中央値は(差分行数で比較した場合で)一桁台とかありますね。

古代、いにしえの時代

少数の大企業だけが使える「基幹系業務システム」というピラミッドみたいな代物を、SEという大工のような人たちを多数抱えたごく少数の「大規模SIer」という集団だけが造れた時代のことです。
この頃はリリースは原則的に「1回」だけでした。「一括リリース」の時代です

「一括リリース」とは

工場の文化が強い時代だったので「完成品を納品する」という思想が強かったのです。
炊飯器や洗濯機を買ったときに、機械のボタンを押しても何も動かず、説明書を見たら「この機能はまだついてません」とかあったらクレーム&返品ですよね?
一戸建ての家を買ったのに幾つかの部屋は内装ができていないとかありえませんよね?
そう言う感覚です。納品した段階で石板のように厚い数十キロにも及ぶ仕様書に書かれた内容はすべて備えられていることが原則となっていました。

なので納品のときのことを「カットオーバー」とか「サービスイン」と呼び、大規模なセレモニーをしたそうです。
きっと生贄の牛とか捧げて作業に従事した者たちは祝祭とかしたんじゃないですかね、知らんけど。

リリースサイズは巨大

みずほのMINORIプロジェクトとか数年単位でかけてましたよね。
作っているうちに機能が時代遅れになったり、リリースしないままOSのサポートが切れるのでバージョン上げ対応とか入っていたらしいですね。
なんなら作っているうちに組織改変や業務フローの変更があって、せっかく作った機能がボツることもあったらしいです。
それでもお金はもらえるんですから、おおらかな時代だったんですね。

少し前の時代

この頃になるとSVNとかgithubとかそういう文明の利器が出てきておりました。
「masterブランチにmerge = ユーザーに該当機能が利用可能になる」という思想の時代です。
なのでリリースの単位とは「機能単位のリリース」となります

「機能単位のリリース」とは

たとえばzennみたいな記事投稿サイトで「記事投稿機能」を作った場合、記事に関する機能に関しては一通り実装された状態でリリースされます。
「記事を投稿」「記事を検索」「記事を閲覧」「記事を編集」「記事を削除」
この全てが揃った状態ですね。CRUD全部を揃えた状態でリリースします。

後の方でもう少し細分化してもう少し細かくリリースした上で、ユーザーに見せないためのリリースフラグ制御を外す、と言うような対応もしていました。
例えば記事機能も「投稿」「検索」「閲覧」「編集」「削除」の単位で分割してPRにして、全部揃ったらリリースフラグ制御を外してユーザーが見られる状態にするやり方です。

それでもリリースサイズが大きかった

「masterブランチにmerge」 = 「ユーザーに該当機能を供用開始する」という思想だったものですから、1つのPRの大きさが変更差分がプロダクトコードだけで1,000行、テストコードも含めれば3,000行という変更もしばしばあったそうです。
「1つのPRを作るだけで1週間かかる」とかよくあったそうですね。
レビュー大変そうですけど、昔のWeb業界のレビュワーの人はレベル高かったんでしょうかね。
その分その時代の人は、大きいPRに関してはだいぶゆっくり時間かけながらレビューしていたそうです。
きっと仕事の進め方もゆっくりだったんでしょうね。

前の時代に比べて

「リリース=納品=完成」という概念がなくなったのがこの時代のリリースの特徴です。
そうですね。外食のメタファを使いましょう。

いにしえの時代の「一括リリース」は「旅館の夕食」です。
食堂に行ってお席にどうぞと案内されると、机の上に「ご飯+刺身+主菜+天ぷら+...+サラダ+デザート」というフルセットが巨大なお盆の上にドン!と載っているわけです。
旅館側は宿泊客が来る前に料理を全部作ってお盆の上に用意した状態で待ち構えるわけですね。
客が来る時間を指定して区切らないといけないし、いずれか1皿の調理が遅れると他が冷めてしまうので大変でしょうね。
全部の皿が揃うまでお盆を置いておくスペースも広く取る必要があります。

少し前まで見られた「機能単位のリリース」は「一品料理での提供」です。
小さい小皿単位で食べられる料理を作り、それが出来上がったらすぐに提供できるわけです。
厳密に客が来る時間を管理しなくて済むのでふらっと来た客に合わせられますし、大食漢にも小食党にも対応できます。
できあがったら即提供!なので提供前の料理を置くスペースも少なく済みますね。
柔軟性もあるし無駄もありません。

じゃあ最近はどうしてんの?

一番大きく変わった概念は「masterブランチにmerge」 = 「ユーザーに該当機能を供用開始する」という概念の否定です。
masterブランチにmergeされたコードは客が利用できる状態にあらねばならない、というのはいにしえの時代から生き残ってきた伝統的な考えに引きずられたもので、実はそれは必須の考えではないんですよね。

先ほどの外食のメタファにもあった通り「客の前に出すからには料理として食べられる状態になければいけない」的な、リリース=客の前に出すという思い込みが伝統的な考えに縛られた人だと強すぎるのだと思います。

またBDUFというほどではないにせよ、着手時点で頭出しの終わったエンジニア複数名でホワイトボードなどの前で設計レビューを行い、大雑把なアプローチのコンセンサスを取ってから着手する感じです。
コードを書き始める前の作業が増えました。
こうすることで効率を上げる仕組みです。

私は1人のメンバーあたり、1日にPRを20個くらい作って欲しいと思っていますがこの高速開発のやり方だと
「E2Eで完成した状態でPRを作る。そこまで作り込むのが実装者の責任」
「コードを書く時間をとにかく増やす」
「レビュワーはリーダー以上の限られた高レベルのエンジニアがやる」
と言うような伝統的価値観とコンフリクトするんですよね。

この方法を使う理由が2つあります。
あるチケットが「数日経っても終わらない泥沼状態」に陥ることを防げるからです

「走り出してから考える」を防ぐ

実際にはコードを書き始めてから考える、を防げるわけです
ある程度以上のエンジニアなら、ある課題を解決する方法はパッと思い付きます。
しかしそれが正しいかどうかは全く別問題なわけです。
筋の悪いアプローチだったことが発覚するのが「コードは書き終わった、ほぼ8割終わった!」というPRレビューのフェーズと言うのは遅すぎるのです。

品川駅からお台場海浜公園駅の移動に例えてみよう

この例で言う「走り出してから考える」と言う失敗はたいてい
「品川駅から飛び出し、まっすぐ東に向けて走ったところ大きい橋が見えたのでそちらに向かった。その橋を渡ってみたところ90分で辿り着けた!(ただし汗だく)」
のような脳筋アプローチをしています。東に向かったら橋が見えたからそれを渡ればいいんだろ? 的な、目の前にある解決できそうな安易なアプローチに深く考えずに飛びついてしまうわけですね。
こう言う失敗をする人はレビューの段階で 「レインボーブリッジって徒歩で渡れたんですね!高くて怖かったけど頑張りました!」 と誇らしげに語ってくれます。
こちらはどういう表情をすべきか困ってしまいます。

及第点の回答は「所要時間と体力と費用の問題を考え、(駅から駅への移動なので)NAVITIMEなど乗換案内で調べた上で、"大崎駅経由でりんかい線" OR "新橋駅経由でゆりかもめ"で移動」です。
所要時間約25~30分、費用は500円弱です。

上の全部徒歩のアプローチを下のアプローチで作り直させる作業が、実際のチームではある程度の頻度で発生してしまいます。
どういう頻度かにもよりますが「自分は一人前のソフトウェアエンジニアだ!」と思っている人間に「いいかい坊や、難しいと思ったら事前に人に相談するんだよ?」と指示するのは現実的ではありません。
上記のケースにおいても
「なんか近そうだし、徒歩で行けそうだな!」「地図見る限り橋があるだろ?」「ほらあった! じゃあこれは難しい問題じゃないな!
と判断する人は後を絶たないからです。
また中堅以上のエンジニアでも何かを見落とすことはあります。
自分が何かを知らないことを知る、いわゆる無知の知を他人の目と脳なしに知覚することは非常に困難だからです。

となるとこうするしかない

「なにをやるにしろ、とにかく他人と相談しろ」
「他人に見せて、それが妥当なアプローチだと相手が言ったらそこでGO!」

これをデフォルトにすれば円滑に回るかもしれません。

あと実は別に経路の最適解があります。レインボーバスを使うと20分220円ですね。
ただこれはレビュワーをつけても「Googleマップとか使えばいい。乗換案内は電車に執着するからクソ」という点を知らないと辿り着けない最適解です。

ただし「他人の目と脳を使ってチェック」は「レビューの高速化」「筋の悪いアプローチの回避」を避けることが目的なので最適解に絶対辿り着ける!的なアプローチではありません。
徒歩90分と言われたら明らかにやべーと気づける確率は高いですが、設計レビュー時に2人のエンジニアの目を通しても最適解に気付けないならそれは仕方のないことです。

もう1つの問題、レビュー地獄

ある日お仕事をしていたら、Slackでメンションされて「このPRのレビューをオナシャス!」と言われました。
リンクを踏んだら変更差分 +5,000行、 -3,000行のPRが出てきました。
一言で言うとレビュワーの脳内に「くっ!ガッツが足りない!」というエラーメッセージが出ます。

「おうとりえあずなに考えてこのPR作ったんや?イチから説明しろや?」
と相手の席まで行って詰めてやりたいですが最近はリモートワークが多めでこれも難しいです。

しかし事前に通知なく1,000行超えのPRを送りつけることは反社会的な行為です。事前通知ありでも大問題です。繰り返すようなら予防拘禁もやむを得ません。

・本当にこんなに大規模な変更入れる必要あったのか?
・あったとして打った手の1つ1つは妥当か?
・妥当だとしても、コミット単位で整理されていても、それが妥当か調べるのがマジ辛い
という問題が目の前に立ち塞がります。
一言で言うと俺がレビュワーなら「うわ絶対見たくねえ」です。

このレビュワーのMPをごっそり削るし、(まあだいたいこうなるが)レビューラリーになったときのコストやリードタイムの遅延は管理職のストレスがマッハになるレベルです。
なのでどうにかしてこれを防ぎたいです。

すくなくとも「いきなり大規模変更が出る」「その打ち手1つ1つを精査する」「(高確率で出る)修正要求の対応」「それによるリードタイムの悪化」の4点セットの連続コンボは到底許容できません

ではどうするか?

最初の実装着手前にきっちり実装者とレビュワーで握っておく、これは基本です。
これは前の品川駅→お台場海浜公園駅と同じアプローチです。
前提知識や設計も含めて握っておけば、あとで揉めることはありません。

あとは打ち手を精査しやすいように細かくバラすだけです。

「未完成」なものをmergeするの?

そうした方が間違いなく開発が速くなります。
数字ベースで見た上で速くなる確信がなければこんなやり方はやりません。

LeanとDevOpsの科学 テクノロジーの戦略的活用が組織変革を加速する はお読みになられましたか?
Four Keysの概念はご存知ですよね?
その前提と数字ベースの比較結果を見た上でこう言う話をしています。

具体的にどうするかと言うと……

おっと時間が足りなくなってしまいました。
(意訳: 続きを書こうとしたら数千文字になりそうだし諦めた)

:bear: でも飲む機会があればその際にお話ししましょう。
あるいはコンサルのご相談も承っております。

Discussion

社内メンバー向けにクイックに仮説検証するような場合(=品質水準が厳しくない)はリードタイムを早めるために必ずしも厳密なレビューが必要ではないので、1000行の差分すなわち反社会的行為とは全く思いません。

また細かい粒度が望ましくとも大きな差分のPRを作ってしまう場合、作成者に何らかのコンテクスト、例えば経験不足やリリースを急がざるをえないなど、があるわけで強い言葉でそれを否定するのはリスペクトに欠けているのであまり同調できませんね。

ログインするとコメントできます