【リファクタリング】人災にならないRuboCop運用マニュアル
はじめに
対象
- チーム開発で組織的にリファクタリングを行う方向け。
技術の行使と段取
ITエンジニア技術に目がいきがち
ITエンジニアは技術やそのベストプラクティスは気にする割に、業務に取り入れるときの組織的な動き方を軽視しているように見えます。
例えば会社の月次掃除や、年末年始の大掃除で考えてみましょう。出社組のITエンジニアならば一度は経験があると思います。机の上のホコリを拭いたり、冷蔵庫の中身を点検したり。リファクタリングでいえば、実際にコードを修正している真っ最中です。
しかし本来の作業はもっと前から始まっています。掃除にかける時間と人員の確保、事前に関係者に向けて連絡などなど、"人"に対する配慮が欠かせません。
段取なしは非効率かつ傍迷惑
掃除を手際よく終わらせることができる。机をめちゃくちゃ綺麗に拭くスキルを持つ。確かに技術があるに越したことはありませんが、それ以上に大切なのは段取です。
事前に何の連絡もなしに、いきなり大掃除を始められては困ります。ホコリが立つわ、「椅子ごとどいて」と言われるわ、とても仕事どころではないでしょう。最悪コンセントを抜いてしまう、なんて事故もあるかもしれません。
自分一人だけのスペースであれば、自分の好きなタイミングでいくらでも時間をかけて掃除してもらって構いません。しかし会社で複数の人が同じ場所を利用しています。自分勝手にリファクタリングをしていい理由はないのです。
企業の本分は営利活動、なるべく本業に影響が出ないように配慮しなければなりませんし、チームのメンバーとの連携は必要不可欠です。幸いRuboCopには並行開発への影響を抑えながら、リファクタリングを進めるための機能が備わっています。
運用マニュアル
手順書ではないので、取り入れられるものからで良いと思います。
先遣隊による影響調査を行う
いきなりリファクタリングを行うのは無謀です。チケット駆動開発であれば、担当者が調査と修正を同時に行うことが多いのですが、この2工程は分けるべきだと考えています。
なぜなら調査の結果を受けて方針を決めるのはチームだからです。詳細をミーティングに持ち込み、チームで合意した上で進めるのが道理。調査と修正の間には話し合いを挟むため、タスクとしては時間が空きます。
また修正を行うのは調査した本人とは限らないからです。チーム全員で一気に行うこともあれば、数名でファイル単位に分担することもあります。担当者や作業範囲ごとにチケットを用意するのが筋です。
- 調査
- 対象のルールをドキュメントで確認する。
- プロジェクトで採用し得る設定と、各設定の場合の影響を調査する。
- 可能であればボリュームや工数も出す。
- 必要であれば修正方針をまとめる。
- 修正
- 実際にルールを変更し、コードの修正を行う。
具体的なケースも合わせて考える
これはどの現場もあるあるなんですが、見切り発車で失敗することが結構ありました。前項の「必要であれば修正方針をまとめる」のところで、特に手動修正のみでしか直せない場合です。
できれば先遣隊はいくつかのファイルをピックアップし、試しに修正を行ってみるのが良いです。ボリュームや工数を出すのに必要ですし。直し方が何パターンかある場合、このプロジェクトではどの直し方が妥当かをチームで検討するからです。
例えばRSpec::LetSetupは、元々のletの書き方がRSpecの想定に沿っていないと地獄を見ることになります。件のケースでは根本的なletの使い方の修正が必要でしたが、見切り発車では場当たり的な対応が目立ちました。ルールの本質やletの基本が頭に入っていないからです。
各自が警告が出ないように直した結果、修正する前より見辛くなっては本末転倒。壊れやすいテストになってしまったこともあります。リファクタリングの当初の目的を忘れて、警告潰しに明け暮れた結果がこれではげんなりです。
内容をチームで相談する
製品によって無効にした方がいいルールや、カスタマイズした方がいい設定がある筈です。既存のルールとの兼ね合いもあるでしょう。過去のプロジェクトの経緯など、一部の方しか知らない事情もあるかもしれません。
何よりチーム開発である以上、コーディングルールについてはメンバー間で認識を揃えておきたいです。全員が納得するまでは厳しいですが、せめて納得できずとも理解できるところまでは話し合いたいところです。
それでもどうしても我慢できないメンバーは出てくるかもしれませんが、最終的に意見が割れた場合はリーダーが意思決定をせざるを得ません。様々な要因が絡んでくるとは思いますが、できれば誰が言っているかより、何を言っているかで決定したいです。
この相談会は実施前にやらないと禍根を残します。事前に一言あるだけで違うのが人間です。いきなりプルリクを出されるよりも、相談という体で、どの方針にするかフラットな状態で、始めるのがポイントです。
できるだけ影響が少ない手段を取る
ありがたいことにRuboCopには.rubocop_todo.ymlを用いた、段階的なルールの適用方法が用意されています。このファイルはコマンド実行で生成することができ、違反箇所はルール毎・ファイル毎に除外されます。
どうしても一括対応が難しい場合は、Excludeのファイルパスを削除しながら少しずつ負債を解消することができるようになっています。
reviewdogで差分のみチェックしている場合も、内容によってはこの方法をおすすめします。いきなり.rubocop.ymlでルールを有効化すると、プロジェクト全体やメンバーが混乱するので、やめた方が無難です。
というのも案件として対応するプルリクに、リファクタリングの修正が混じると差分が見辛くなります。ほぼ全てのコードに影響があるケースだと、毎回リファクタリングが付いてきます。まだAutocorrectで直せるものなら良いですが、手動修正が必要だと案件の見積にも影響が出ます。
時期や人員を調整する
軽い修正であれば即日でも問題ありません。問題は重いかつ広範囲なリファクタリングを行う場合です。経験上、この手の修正は期間を決めて一気に全員で消化する方が、結果的に低コストかつ事故る危険なく終わらせられます。
これは人伝に聞いた話なので実施経験はないのですが、四半期ごとに事業計画を立てた上で、予定より早く終わった場合には、まとまったリファクタリング期間とする現場があるそうです。これは非常に上手いやり方だな、と感心したのを覚えています。
どうしても機能改修や新規開発と並行して行うと、ブランチ間の競合解決やマージのタイミングなど、考えることが増えます。他メンバーの作業に対する関心が低い現場だと、競合の発生を予測して対処する習慣がないため、いつも解決するのは特定のメンバーだったりします。
これでは上手くいきません。酷いときは競合解決をミスって、ゴミがコミットされる・障害が発生するなど、目も当てられないことになります。
どうしてもまとまった時間が取れない場合は、どの辺りのコードを触るのか、そのマージ時期がいつ頃かを共有できると、他のメンバーも対応しやすいです。必要であればタイミングをずらすことも考慮に入れ、無理に強行することはやめましょう。
おわりに
本記事の内容はソースコードやリファレンスには書かれていませんが、チームとしてリファクタリングを進めていく上で重視すべきポイントだと考えています。
既にできているチームは問題ないのですが、できていないチームは気付かないうちに生産性を低下させています。それは理解も納得もしてないエンジニアのモチベーションの低さだったり、計画性のない断続的なリファクタリングによる案件効率の低下だったりに現れます。
開発チームとリファクタリングチームを分けたいとか、リファクタリングをもっとやりたいといった声はちらほら聞きます。しかし詳細を伺うと、実際には本記事で挙げたような人への配慮や計画性が欠けており、うまく運用できていないケースが多いのです。
もし思うところがありましたら、この記事をきっかけに改めてチームとしてのリファクタリングの進め方を見直してみてはいかがでしょうか。
Discussion