読者です 読者をやめる 読者になる 読者になる

「現状○○なんだけど、いずれxxしたい」という時のアプローチ

はじめまして、こんにちは。adingoFluctの開発に携わっているmasartzです。

結論

Q「技術的には全然難しくないけど、面倒で地味なお仕事をどうやってやるか?」
A「いつか一気に片付ける!は片付かないので、最初だけ気合入れて・あとは粛々と進めましょう」
というお話です。

なぜxxしたいか?

Fluctのパートナー企業様向け管理画面はPHPのwebアプリケーションとして実装されています。 Fluct開発チームではgithub のPull Request(以下PR)によるコードレビューが導入されており、エンジニア同士の活発な議論が行われています。
この中で、以下のようなやりとりがありました
レビューア「ここの部分のコードはPSRに則った形で記述してください」
     「この括弧は改行せずに同一行に記述する形でお願いします」

この状況を見て、
「この作業は機械的に、チェック/修正 できるようにした方が圧倒的に良いな」
と思いました。
この手の指摘を人間が行うと、漏れも出るしコストもかかります。

幸いPSRに準拠するためのツールとして、「PHP Coding Standards Fixer」があるので、これを使えば解決しそうです。 (注:「プロジェクトにある種の規約を設けるべきか」については本エントリでは言及しません)

How To Use php-cs-fixer

説明するまでもないレベルで、composer で入れられます。 composer.json に以下のように追記してinstallするのみです。 通常は開発時のみしか使わないものなので、require-dev句が良いと思います。

"require-dev": {
    "fabpot/php-cs-fixer": "1.6"
},

コマンドも非常にシンプルで以下のように打つだけです

php-cs-fixer fix ./lib --level=psr2

どうやって導入するのか?

これを導入して、コマンド一発でそれが適用される形になると コードを書く側も余計な事を意識せずコーディングできますし、 コードを読む側も「コマンド一発かけといてください」と言うだけで済んで良さそうです。

しかし、現状のリポジトリ内にはツール適用した場合に改変されるファイルが大量にあります。 この場合、例えば1ファイルだけ修正して、コマンド実行した場合 「全体差分で見るといじってないファイルのdiffまで出てエラいことに!」 という導入障壁がありそうです。
またそのような障壁があると、

  • どこを自分が修正したかわからないから困る!
  • レビューする時に関係ないdiffがあって見づらくて困る!
    • -> 今回はコマンド実行しないでおこう

という風になりがちです。
導入するためにはまず 「リポジトリ全体が、fixerが適用されている状態にある」という前提を作る必要があります。

こんなことやっています

という訳で最近の私のPRの一部がこちらです。

f:id:voyagegroup_tech:20150430101604j:plain

「既存のリポジトリに対するphp-cs-fixer適用」の1個だけのPRじゃないのは何故だ?というツッコミもあるでしょうが、

  • PR をレビューする側の確認コスト(大量すぎるとしんどい)
  • 障害時のビジネスインパクト

を考慮して、小分けにしております。

Q「基本的にコマンド適用でバグる事も考えづらいし、それを律儀に目視確認する必要はあるのか」 については
A「地味な改善系は事業収益に直結する訳でもなく、逆に障害を出すとその後の作業にも影響するため、より慎重にやるべき」 と考えています。
「利益にならない作業をして、しかも障害出して、あいつ何してんだ」と思われないようにしましょう、という話です。

このPRの一覧は現在も継続中であり、全体適用にはもう少しかかります。 その間のメンバーの並行開発はどうしているのか?という点については fixer適用は何も意識してもらっていません。 つまり、この瞬間もfixer適用外のコードが生まれつつあります。 それを許容しつつ進める事が重要であり、そうでないと

  • 他の開発者がこの地味な適用作業をしたり、(未適用リストっぽいものをExcelで管理とかして、分担しちゃったりする)
  • あるいは私の適用作業を待つまでその部分の開発を止めたり、

と言った事態になります。 それよりは

  • masartz   :適用度合いを n% -> 100%にする
  • 他の開発者 :ガリガリ新規開発をして、リポジトリファイル数を5〜10%増やす

で作業した結果、「両者が終わった時に適用度合いは90% 〜95% 程度になる」とかで良いと思っています。 100%ではない訳ですが、この残り5% 〜 10%は追加作業としても苦ではありません。 逆にこれを許容しないと「いつか一気にやる!」パターンになり、ついついやらない方向に倒れるものです。

長くなりましたが、タイトルに当てはめると
「現状スタイルが統一されてないんだけど、いずれ統一したい」という時のアプローチ方法として、
「いつか一気に統一する!は統一できないので、現状分だけ気合入れて直して・新規分は粛々と進めましょう」 となります。

その他の例

今回は『規約適用』でしたが、例えば
「現状は、不完全でたまに落ちるテストしかないけど、いずれもっとテスト増やして全部をJenkinsでpassさせたい」 という場合にも同じようなアプローチです。
この場合、

  • 新規にテスト書いても既存のテストのせいでJenkinsがパスしない(ことがある)
  • ひとまず新規開発はテストを書くけど、Jenkinsは稼働させない
  • 既存のテストはいつか一気に直す
  • 直ったらjenkinsを稼働させる

こんなアプローチでいこうとすると、

  • 元から落ちる状況なので、手動でfull testを回す作業をついつい疎かにする
  • 自動実行される恩恵も享受されないからそもそもテストを書かなくなる
  • 安定しないテストやコードがより増えて行く

というスパイラルになります。

この場合は、暫定的に現状落ちるテストを整理する必要がありそうです。

  • テストそのものを消す
  • チケットなどのissue管理をした上で、テストを無効化(コメントアウト)する
  • 簡単に直せるものは直す

などをまず行い、質はどうあれ、ひとまず「full-testがパスする」状況を作ります。 「full-testがパスする」という前提ができたら、テストが落ちた際には必ず修正するようにします。 これによって、少なくとも「安定しないテストやコードが増えて行く」事はありません。
あとは、暫定対応した部分を逐次取り組むのみです。 テストを消した所には追加し、コメントアウトした所は有効化して正しくテストを書くようにする、です。

チケットよりも、リポジトリ内にblacklist.yamlなどを置いて、full-testがそのリストのファイルを除外しつつ処理する、などとしておくとリポジトリ内で管理が完結できて良いでしょう。

まとめ

改善タスクは、営業や企画の方はもとより、他の開発者にも気づかれないくらい地味にやっていけると幸せです。 逆に他の人が意識している場合は、改善作業が必要な何らかの問題が顕在化してしまっている状況とも言えるので、そうならないうちに作業できると良いですね。 大したコードも載せずに、講釈ばかりが多くなりましたが、何かの参考になれば幸いです。