はじめまして、PR TIMESの開発本部でインターンをさせていただいている神戸と申します。
インターンでの業務としては主にPHPのレガシーコードのリファクタリングを行っています。
はじめに
自分は情報系の学校に通ってはいますが、授業で触る程度の規模でしかプログラミングをしていなかったです。
そのため、大きなプロダクトに関わったことはなく、PHPには触れたことすらありませんでした。
さらに、PR TIMESの業務では、はじめて扱うものばかりでした。
PHPにおいてはIDEとしてPhpStormが優秀であるため、はじめてVisual Studio Code以外のIDEを使って違いに苦戦しました。
また、そもそも社用PCがMacbookなので普段使いのWindowsPCの感覚でキーボードを扱うことができずに苦戦しました。
そのため、最初はSlackでチャットを打つのにも苦戦しながら業務を進めていきました。
そんな右も左も分からない状態でもインターンを通して関わる経験ができて、prtimes.jpで自分の触ったコードが動いているというのはとても良い経験になったと思います。
PR TIMESについて
PR TIMESでは現状は古いバージョンのPHPでの開発が行われています。
しかし、バージョンが古いままだとモダンなライブラリ等を追加する際の障害になっているそうです。
PHPは歴史の長い言語なのもあり、長い歴史の中で行われた関数や記法の削除の影響で古いバージョンを簡単に最新に上げることができません。
PR TIMESにおいては、Deprecated(古く、非推奨)な関数の書き換えを大量に行う必要がありました。
また、PR TIMES自体が長く運用されているプロダクトなのもあって、現在は使われていないコード、いわゆるデッドコードもありました。
このDeprecated(古く、非推奨)な関数の書き換えとデッドコードの探索が最初の業務でした。
Deprecated(古く、非推奨)な関数の書き換え
まず、PhpStormの全文検索と正規表現を利用してDeprecated(古く、非推奨)な関数を探索しました。
PhpStormではShift＋command+Fで全文検索が可能です。また、全文検索画面の右上にある .*(Regex)を有効にすることで正規表現を使用しての検索が可能になります。
正規表現を使用することで、複数条件での検索が可能になるため、何度も検索窓に入力する必要がなくなりました。
例えば、PHP７で削除されたereg関数とeregi関数の両方を探したい場合は、\bere(g|gi)\b\(とすることで検索が可能です。
次に、XdebugというデバッガをPhpStormに導入しました。
ブラウザに拡張機能を追加することで、実際の画面の操作しながらデバッガを刺すことが可能になります。
以下のサイトを参考にしました。
https://www.luftgarden.jp/blog/102/
該当箇所や前後の条件分岐に対してブレークポイントを設定後、ステップ実行で挙動を調べ、使用されている変数に入ってくるデータの確認をします。
その後、Deprecated(古く、非推奨)な関数を使用せずに書き換え、もう一度デバッガを刺しながら挙動と変数に入るデータに変更がないかの確認を行いました。
PR TIMES内部のこのようなDeprecated(古く、非推奨)な関数を使用している古いコードの箇所ではUnit Testが存在していません。
Unit Testがあれば、今回自分が行ったデバッガを使用しながら挙動変更がないことの確認という一連の流れと同一の作業を自動で行うことができます。
今後これらの箇所にもUnit Testを追加するための準備として、単に関数を置き換えるだけでなく、型を厳格にする等のリファクタリングを並行で行うこともありました。
mainブランチへの追従
学業もあるため、インターンは月金の週2勤務です。
PR TIMESは小さい更新をどんどんmainブランチにマージしていく開発手法を取っているので、3日も開くとmainブランチと自分の作業しているローカルブランチの差がとんでもないことになります。
毎回git rebaseでmainブランチに追従し、適宜コンフリクトを解消しながら開発を進めました。
プルリクエストの作成
作成した変更はQAチームに確認してもらい、QA完了後にデプロイを行います。
関数の書き換えは単一の機能追加と比べて、変更が広範囲です。
変更した範囲と、実装が間違っていた場合に影響が起きる可能性のある範囲をプルリクエストに詳細に記述し、レビューを受け、さらにQAチームに確認をお願いしました。
本番環境を壊した話
業務にも慣れてきた頃、本番環境を壊しました。
QA完了後から時間が経っていたため、デプロイ前にgit rebaseでmainブランチに追従するようにしたところ、いつものようにコンフリクトをしました。
それもコメントの有無だけであったので、HEADを受け入れる形で書き換えました。
コメントの有無だけという慢心もあったのかもしれないです。
いつもならコンフリクト解消を行なった後に行うはずのE２Eテストを走らせずに、そのまま本番環境にデプロイをしてしまいました。
コンフリクトの解消をミスしていました。
prtimes.jpのサイトが繋がらなくなりました。
会社のSlackですぐに報告が入りrevert作業を行いました。
現在はデプロイを高速化するプロジェクトが進行中ですが、当時はデプロイに時間がかかるのもあって元に戻るまで15分程かかりました。
その間、部屋には暖房がついてるはずなのに足が震えていました。
テストの重要さを身をもって経験しました。
冷静な対応を教えてもらいながらrevert及び障害報告をすることができたことは良かったと思います。
これをきっかけに、本番リリース時に以下のスクリプトが追加されました。
- 変更されたファイルリストを取得する
php -lで変更されたファイルにsyntax errorがないか確認する
自分がコンフリクト解消ミスでPR TIMESの本番環境を壊した最後の人間になりそうです。
最後に
Deprecated(古く、非推奨)な関数の書き換えは無事完遂することができました。
業務を通して、テストの重要さや後からバージョンを上げることの大変さを経験することができました。
現在は、New Relicにおいて発生しているNoticeやWarningなど、停止まではしないが良くないコードを改善する業務を行ってます。
型を厳格にする手法であったり、早期リターンでネストを浅くして読みやすくしたり、ただ動くだけではなくコードの書き方は学ぶべきことが多いと感じています。
インターンで生かした経験を今後の開発に生かしていきたいです。
