PR TIMESにおけるリファクタリングデー

こんにちは、業務委託でPR TIMESにJOINしているuzulla (”うずら”, twitter, GitHub)です。本エントリではPR TIMESで行っているリファクタリングデーについてお話したいと思います。

目次

リファクタリングデーとは?なぜ必要か?

PR TIMESは歴史あるサービスです。サービス・機能は追加され、削除され、結果古いコードが大量に存在します。今後もスピード感を維持しつつ成長を続けるためにはそれらの整理・改善・削除、つまりリファクタリングが重要なことはいうまでもありません。

日々の業務においてリファクタリングが行われていれば問題ありませんが、日々サービスの成長にフォーカスしていくとやはり後回しになりがちで、タイミングを失います。

その為に「リファクタリングデー」という「リファクタリング作業を定期的に実施する日」をとりきめて実施しています。現在は月に一回程度開催され、3回が経過した所です。

社内Notionより

リファクタリングの難しさ

リファクタリングは基本的に内部的な改善です。内部的な改善というものは言うは易し行うは難しの典型例と思われます、なぜなら前と同じ挙動ができているか、外形を変えない必要があるからです。

しっかりと自動テストが構築されたウェブアプリにおいては、E2Eテストやユニットテストによってそれが担保され、ハイサイクルで改善を行う事ができます。しかしPR TIMESではそれらの整備は途上です(言い換えれば、整備されつつあります)。よって、開発者やQAチーム(テスト専門のチーム)がひーこらといいながら、しっかりとチェックせざるを得ません。

その状態で、なにかちょっとしたリファクタリングをする度に全部を見る…悪夢です、たいへんです、倒れてしまいます。そこで、我々のリファクタリングデーです!

リファクタリングデーでは「単体の機能それぞれがこわれていないか?」という通常のQAではなく、「全部がこわれていないか?」を見るという非常にヘビーなテストを行っています。よって、私の中では「QAチームのみなさん!負荷をかけてごめんよ!」デーでもあります。リファクタリングの主役はQAチームである!!

そのために、リファクタリングデーマスターを努めている私は(自分もガンガンリファクタしますが)どのように「スムーズにQAが実施できるか」を工夫・コントロールしています。

さて、前置きが長くなりました、実際にPR TIMESにおいてリファクタリングデーをどのように行っているのか簡単にご紹介したいと思います。

チケットの起票

なにはともあれチケットです、PR TIMESではJIRAをつかっているのでJIRAに起票します。これは一週間前くらいに建てると私の気分が盛り上がります。

JIRAチケット(本来はサブタスクは空から始まる)

すべてのリファクタリング作業は、このチケットを親として子タスクとして登録します(多くのイシュー・バグトラッカーでは親子がつくれると思いますが、まあ置き換えて考えてください)。これによって内容を一括して管理しています。

リファクタリングデーブランチの作成

通常のgitにおける開発では、main等のブランチから作業ブランチを切り、開発をして、テストし、mainブランチにマージし、テスト・デプロイします。

しかし、リファクタリングデーにおいてはその手法は難しいと考えています。

  • 「全部テストせよ」なので、個別にテストは大変すぎる
  • 全部テストするなら、1本化されているとQAが省力化できる
  • (原因がわからない時に)Merge&Revertは一括で行いたい

そうはいっても、リファクタブランチ一本によってたかってcommit作業することは現実的ではありません。force pushできないなどありますからね。

なので「リファクタリングデーブランチ」をemtpy commitで事前作成しておき、そこから皆がブランチをきり、そしてめいめいでそのブランチにむかってMergeしていく。そして「一本の巨大なリファクタリングブランチを完成させる」という戦法を取ります。

ツリーができ、マージされていく様子

これが本手法のキモです。

もちろん、問題があれば随時そのプルリクはRevertなどされながら、リファクタブランチが完成し、そして最終的にmainに取り込まれていきます。これは後で詳細を書きます。

オンボーディング資料の作成

これはもう何回かやっていてテンプレになっているのですが

  • 開発者がなにを準備するか
  • どうやって作業をすすめるか
  • どこのチケットを親とするか
  • どこのブランチから切るか

などを記述した説明書があります。それを作成・更新します。

社内Notionより

他にもたとえば、「リファクタリングとはなにを指すか?」などが記載されています。我々は現在デッドコード(使われていないコード・ファイル)の削除に主眼をおいていますので、そういった意識の共有のためのドキュメントを記載したりしています。

オンボーディング

リファクタリングデーは特段全員参加ではないので、初めて参加する人もいます。なので、今の所毎回流れを説明するテレカンを実施しています。

その時に質問がないか?あるいはどうやってデッドコード(使われていないコード)を探すのか?などをエディタをシェアしながら紹介したりします。

Meetで録画しています、無編集、手間はかけません。

開発者は事前にリファクタリング対象を探しておく

重要なことなのですが、リファクタリングデーにリファクタ対象を探すことは全く推奨されません。というか、無駄です。

リファクタリングデーは「テストをしきる」ことの優先順位を高く据えており、事前に安全にできるような「探す作業」のを当日にやる必要はありません。日々「ここがリファクタできるな」「ここ、デッドコードだな」と目星をつけます。

普段からリファクタ対象を考えてチケットにしておく。それをリファクタリングデーにおいては小タスク(サブタスク)に変換して実行する。そのような効率化が重要です。非常に重要です、本当に重要です。何度も書きますが、これで成果の量が変わると行っても過言ではありません。

探すこともリファクタリングの一部です。そういう意味では日々リファクタリングはできるのかもしれません。

これで準備は完了です!

当日の流れ

さて、ついに当日です。まずリファクタリングデーマスターの私がやることは、リファクタリングデーブランチのrebaseです。当然ながら前日まで普通に開発が走っているので、mainブランチに追従する必要があります。(実際には前日の就業時間後にやってます)

そして、Slackなどで「今日はリファクタリングデーです!!」とたからかに宣言しましょう、カーニバルの開演です!!

時刻ピッタリに投稿するために、予約投稿を使ってもよいと思います

開発、プルリクとレビュー

ブランチをきり、修正をし、プルリクを作ります。この時、「どのあたりに影響がでるか?」を考えてQAチームに役立つ情報を記述できるとベターです(かけないような広い範囲もありますよね、それはしかたない)。

開発や簡単なテストはローカルのDocker環境でもおこないますが、より詳細なテストをおこなうためにはステージング環境も活用します。ただ、リファクタリングデーでは普段よりもより大勢が同時に利用したいケースが発生します。

そこで先日このテックブログでも紹介された、柔軟にステージング環境を生成できる仕組みが活用されています。こちらがなければリファクタリングデーは開催が検討されなかったことでしょう、ぜひ以下の記事も御覧ください。

あわせて読みたい
1台のサーバーで複数のステージング環境を同時に使えるようにする
1台のサーバーで複数のステージング環境を同時に使えるようにするこんにちは、インフラチームテックリードの櫻井です。今回は1台のサーバーで複数のステージング環境を同時に使用できるように設定を変更したので、その方法について紹介...
1台のサーバーで複数のステージング環境を同時に使えるようにする

そして、リファクタリングデーでもレビューはします、ただ、普段よりサラっとしていますね。議論が必要なほど複雑なリファクタリングはリファクタリングデーでは避けるべきだと考えています。

PRの例

逆に、しっかりテストをするのだから、「いけるかな?」というものでも(それがリファクタとして意味があるなら)通すようにしています。

なおレビューが通ればどんどんリベース&セルフマージしていきます。さっさとしないとコンフリクトするので、さっさとやっていきます。

なお、「レビュアーだれにしよう…」と迷うことがありますよね、そういう時は問答無用に私にレビューを投げてもらうことにしています。私はそれのお返しとして「じゃあこれお願いしますね!」と返しています。

マージウインドウ

突然ですが、私のような気の早い人間(なお、私は完全裁量労働人材)は、前日のリリースが終わった後にリファクタブランチをrebaseし、Force pushした後、さっさと作業を開始してしまったりします。

他の開発者のみなさんも、検討をつけている所はシュッとコミットしてPR化し、マージされていきます。

するとどうなるか、リファクタリングデー当日の昼すぎには「第一弾」みたいなものが完成しています。これを暫定的に「午前分」とよびますが、12時過ぎごろに仮想的なマージウインドウとして(特になにかするわけではない)、13時ごろよりQA環境にてテストを行っています。

これにより、QAチームの作業が二倍になってしまうとはいえ、「午後分」が完成するまでQAチームが何もしないのはもったない(?)ですし、発生したデグレ・バグが早くみつかれば当日救うことが可能です。よって、午前分で一度テストするのは有用な方法だと考えています。

そして、本当の最後のマージウインドウは何時までかというと、15時です。

早いとおもいますか?いえいえ、QAチームがテストをちゃんとやって、Revertし、デプロイを考えるとこれはギリギリと言えます。15時頃(頃です、ある程度合わせます)にマージウインドウをうちきり、16時までにQA開始できる、これが目指しているスケジュールです。(まあ、割とズレます。これはチケットをみながら「進捗どうですか!」と声掛けを私がしています)

結合テスト・QA

リファクタリングデーブランチが完成したら「QAチームの方!お願いします!」となり、そこからはQAチームがリファクタリングデーブランチをQA環境で全力でテストすることになります。

これは私の範疇には入らないのでサラッと書きますが、普段のQA手順書に則って、testrailsで管理しつつされているとのことです。

運が良いことに(?)、私の経験から言えばPR TIMESのQAチームは非常に優秀です。このような「めちゃくちゃ荒っぽい方法」がコストを無視すれば「できちゃう」のです。でもより高みを目指していきたいですよね(と、いう流れで、私はPR TIMES内のテスト自動化プロジェクトにも参加しています)。

リリース

QA環境でのテストが完了したら、本番リリースです!いつもどおりリファクタリングデーブランチをmainブランチにマージし、リリース作業を行っていきます。

しかし…いくらQA環境が本番とそろえられていても…本当のエラーは本番にしかでないことが多々あります。というか出ますね…。

過去3回リファクタリングデーをやりましたが、ついに先日、本番でデグレがみつかることがありました。原因はテストケースの網羅漏れだとか、リファクタがビッグバンすぎるとか色々意見はあると思います。でもそれはやらないとわからないし、やることで「ここが漏れていたね」とわかるきっかけになります。粛々とリバートしましょう。

しましょう、と書いておいて何ですが、まあ…人間なので悪あがきはすることはあり、リバートのリバートをしてみることもあります。色々な詳細は説明が難しいので避けますが「表示に関係する箇所のリファクタリングが、バッチに影響して静かにコケていた」みたいな難しい場面では以下のような事態が発生することもあります。うーん(笑)

さておき、万事問題なければ!(あるいは、リバートのリバートのリバートのリバートをデプロイして)リファクタリングデーは完了となります!お疲れ様でした!

得られる物・振り返り

誰でもわかることですが、同じサービスなら、複雑度が減れば減るほどよいのです、デッドコード削除やリファクタによってどんどんと「なんなのかよくわからないファイル」がへっています。これは説明するまでもないリファクタリングのメリットです。

また、大規模なコードはどんどんと「フレームワーク」化していきます。暗黙のお作法、たとえば必ず入っているけど意味があるかわからないコピペコードが増えていきます。

そして、本件のような「リファクタリングデー」をやれば、自分が普段読まないコードをどんどん読む事になり、「単なるまじないだった」とか、「理解が不十分だった」とか、「古いのでいらないとおもったものがすごく重要だった」などの気付きにも繋がります。よって、新人ほど、あまり普段触らない人ほど気軽にリファクタリングに突撃してきてもらえると嬉しいなと思います。駄目でもRevertすればいいんだからね!

あと、まだあまりワークしていないのですが、振り返りは重要です。新しくリファクタポイント、テクニック、疑問点などを持ち寄りたいところで、ここは今後の課題だと考えています。

ただ、これが「面倒」になってしまうとやる人が減ってしまうので、色々考えています。

そうそう、具体的なコード増減量成果も見たいですよね、過去3回の成果は以下の感じです。機能を変えず、コードドロップだけならなかなかだとおもいますがどうですか?

(なお、Add 行は、大体コードフォーマットですね)

なお、これ以外にも「一人旅リファクタリング」と称して私は週一程度でリファクタリングデーをしていたりしますし、日々PR TIMESのコードを削り飛ばしています。

まとめ

「リファクタリング」は開発なので、無論開発者も必死に作業をしますが、私が思うにリファクタリングデーの主役はテストであり、QAチームです。これを是非皆さん覚えて帰ってください。

もしみなさんのサービスがうまく動いているなら、テスト・QAチームさんの尽力の賜物です。

なお、PR TIMESのQAチームは現在自動化を推進しており、今後テストは開発者自身が行うようになり、QAチームはテストツールをメンテや「人にしかわからない細かい所」に注力する業務にコンバートするのではないかと思います。そうなればきっと近日中にはもっと開発者がわがまま(?)にできるようになるはずです、それを目指していきたいですね!

PR TIMESはプロダクトを改善し続けるために日々邁進しています!「リファクタリングデーという荒々しい手法」があったね、と笑って言える日のために頑張っていきたいと思います。

現場からは以上です。

この記事を書いた人

Jack of all trades, master of none.

目次
閉じる