はじめに / これはなに?
この記事は、WordmoveというWordpress用のデータ移行ツールに、プルリクエストを出してみた際の記録です。
※Wordmoveについては、以前このような記事を書きました。
とりあげること
ことの始まりから書くと長くなりますので、まずは端的に行ったことを。
- なぜこんなことをしたのか
- プルリクエストを出すまでの準備段階
- プルリクエストを出す際の工夫
- 返事をもらうまで
- コードレビューしていただいたこと
題材決めや、こういった活動の理由は後半に書きますので、もしご興味持っていただけたら、眺めて見てください。
- なぜWordmoveにしたか
- 取り上げる題材をどうやって決めたか
とりあげないこと
- Wordmoveそのものについての説明(上記の記事をご覧くださいませ)
では、以下に書いていきます。
自分以外のリポジトリにプルリクエストを出したい!
実は現職の社内では、「社内版OSS Gate」という活動があります。1
毎週の業務時間のうちの1時間、テーマを決めて各自好きなOSSへの貢献に充てようというものです。2
もちろんコードだけでなく、翻訳、紹介記事、知らないものをまず使ってみる、なども構いません。
各自、もくもくと作業ではありますが、進捗を励まし合ったり、誰かのプルリクが完成したりマージされたりしたらみんなで喜ぶ、といったことをしています。
一緒の時間を「OSS活動」として共有することで、孤独感も達成感もわかちあえるので、モチベーションが上がります。
毎週のお仕事のうち、わたしの非常に好きな時間です!
さて、わたし自身は個人のリポジトリを細々とメンテナンスしているのですが、どうしてもセルフレビュー、セルフマージになってしまいます。
できればこの「会社の時間の枠」の中では、自分以外のリポジトリにプルリクエストを出して経験を積みたい...と考えました。
実際の題材選びについては、後半に記載となりますので、続いてプルリク作成からのお話を。
プルリクエストを出すまでの準備段階
こちらは、わりとお約束通りの流れで。
- 元のリポジトリをフォークする
- 手元にcloneしてくる
- masterブランチからトピックブランチを作成する
- 空コミット(Empty commit)をして、自分のリポジトリにpush
- トピックブランチから自分のリポジトリのmasterへのプルリクエストを作成する
まだ空コミットだけでもプルリクエストを作成したのは、以下のようなWork in Progressパターンを使って進めたかったからです。
参考: Work in ProgressパターンによるPull Requestを利用した開発フロー
ただし、まずは「自分のリポジトリへのプルリクエスト」として作りました。
やることを書き出してみる
なぜ自分のリポジトリへ、Work in Progressパターンで進めてみたかというと、以下の点からです。
- そもそもうまく達成できるかわからないので、確認ポイントを設けて少しずつ進めたい
- 1〜2時間くらいで切りのいい単位でやることを決めて少しずつ進めたい
- このプルリクエストの作業を通して、「自分ができるようになりたいこと」「覚えたいこと」を書き出しておきたい
- 学習を兼ねているため、途中途中で調べてわかったことを書き留めておきたい
社内OSS Gateの枠の中で、切りのいい単位で作業したいこともあり、今週はここまでわかればいい、次はここまで実装したい...のような感じで進めていきました。
空コミット時のプルリクエストの記載内容
実際のプルリクエストの画面
こんな感じで、Markdownでチェックリストを作って進めていきました。
また、プルリクエスト自体はこちら。
なかなかモチベーションが続かない性格なので、小さく分けて達成感を感じられるようにしていきました。
自分のためのプルリクエストなので、コメント欄は調査結果や動作確認の内容を記載して進めました。
なんとか形になってきた
Wordmoveの良いところは、Contributor向けのガイドや、テストコードがしっかりついている点でした。
- https://github.com/welaika/wordmove/blob/master/CONTRIBUTING.md
- https://github.com/welaika/wordmove/blob/master/CONTRIBUTING.md#test-wordmove
最低限、オリジナルのリポジトリ側に出したプルリクエストを見てもらうためには、CIでのテストが通らないといけません。
既存のコマンドラインオプションの動作、出力に対するテスト(spec)がきちんと書かれていたので、同じようにわたしもspecを書いて、rake コマンドで確認しながら作業をしました。
本体の調整より、テストコードの準備や調整のほうが長かったかもしれません。
それでも、なんとか形になったので、いよいよオリジナルのリポジトリ側にプルリクエストを出すことにしました。
プルリクエストを出す際の工夫
満を辞して?実際に出したプルリクエストはこちらです。
第一の関門は、プルリクエストを作成した後、CIが通るかどうかということでした。
CIによるテストだけでなく、Rubcopというツールを使って、「書き方」に問題がないかということもチェックされます。
このため、プルリクエストを出す前にはローカル開発環境でなんどもチェックしています。
つつがなくプルリクエスト作成後にCIのステータスがグリーンになった時は、本当に安堵の気持ちでした。
この点も含め、とりあえず以下揃えてプルリクエストを作成。
- 怪しくても頑張って英語で書く
- きっかけになったissueを引用する
- 利用例を添える
- テストコードの出力結果を添える
あとはお返事が来るかどうか....。
返事をもらうまで
社内OSS Gateのチャンネルにも報告したところ、やはり難関は、「プルリクエストに気がついてもらえるか、お返事がもらえるか」という点でした。
そういうわたしも、個人リポジトリにプルリクエストやissueをいただいた時に気が付くのが遅くなってしまうことが何度もあったので、ここは「日頃の行いか...」と思って待ちました。
最初にお返事をいただくまで、2週間ぐらい経過したでしょうか。うれしくて改めて報告すると、みんな祝ってくださいました。嬉しい!!!
コードレビューが始まった!
今思うと突然過ぎて申し訳ないくらいなのですが、Alessandroさんがレビューに名乗りをあげて下さいました。
それまで、Wordmoveのリポジトリを抱えているOrganizationがイタリアだということも知らず...。
にもかかわらず、最初のコメントは割と丁寧に(わたしからするとガチで)レビューしますね、というものでした。
本当にありがたいです。特に、以下の一文。
If you feel like my review is blocking your contribution (too strict? too verbose? no time to follow up?), tell me sincerely your feelings and we'll find a way to go straight to the precious merge.
(おおざっぱな訳: レビューが厳しすぎるとか、煩わしくて、「どうもプルリクエストが受け入れられてないのかも?」と感じたら、率直に伝えてください。せっかくいただいたプルリクエストをできるだけそのままマージできるようにしますので)
受け取り方はいろいろあるかもしれませんが、とにかくわたしには非常に嬉しいことでした。
Contributor向けのドキュメントも丁寧だったので、コードれビューに関して非常に配慮されている方なのかな...という印象を持ちました。
そんなところで舞い上がっている中で、最初の一個めの指摘は「スペルミス」でした....。
こんな内容をやりとり
- スペルミスが何回か...(お恥ずかしい)
- 例外処理の仕方
- アクセサの位置とか
- いろいろあると思いますがprivateにしない、クラス宣言の直下がいいな、とか
- メソッド名
- 他のクラスにならってメソッド名をつけたけど、役割的には「出力」だから
print
が良いよね、とか
- 他のクラスにならってメソッド名をつけたけど、役割的には「出力」だから
- 短くは書けるけど、読みにくさがある箇所(たとえばこんなコード)
''.tap do |retval|
vhost_list.each do |entry|
retval << "#{entry[:env]}: #{entry[:vhost]}\n"
end
end
わりと安易に名前付けをしていたところは、クラス名にしてもメソッド名にしても、「こうしたほうがいいよ」というコメントをいただいております。
マージしてもらえた!!
なんだかんだで指摘をいただいたところを直して、再push。
最初にプルリクエストを出してから20日くらい経過していましたが、幸いにもApproveいただき、マージの運びとなりました!
そのころSlack上では....。
わたしのプルリクエストと合わせて、新機能が追加されたことで、Wordmoveもメジャーバージョンアップとなっています。
そのあとも継続的にバージョンが上がっているのが、すごいところ!
補足編:モチベーションとか理由とか
なぜWordmoveにしたか
もう6年以上前になりますが、当時お仕事でWordPressを使っていたため、wordmoveを使ってWordPressサーバのデータを開発機に再現という記事を調査がてら書きました。
この記事、現在も時折ストックやイイねをいただくことがあり、わたしのQiitaのContribution数の中では割合が高いものになっています。
実際凄いのはWordmoveであり、わたしは紹介しているだけなので、なにかお礼にでもなるようなことがしたい...というのがきっかけでした。
題材はどう決めたか
プルリクエストの前に、まずは題材決め。
自分のリポジトリなら思いついた時にパッとやってみることができるのですが、なかなかそうは行きません。
社内OSS Gateの時間も、全てがコーディングではなく、各自題材決めの時間とすることも多いです。やはり、日々のissueの観察、もしくは自分で使い込んだ中からの発見でないと、すぐには題材が決まりません。(ちなみに、それも大切な時間と許容される雰囲気があるので、ここは有り難いです!)
そのパターンに倣って、Wordmoveと決めたあと、まずはしばらくissueを眺めることに。
それなりの数はあるのですが、バグ対応だとWordmoveとWordpressの環境を構築して検証しないと取り組みにくい...。
そんな中で見つけたのが、今回のプルリクエストのきっかけのissueでした。
- コマンドラインで一覧を出力するだけ、さらに新機能なので他に影響が出ない
- Wordmove単体でコーディングできる
ということで、試してみることにしました。
覚えたことは?
このissueに決めた理由は、他にもあります。
- gemとして利用するものであること
- そもそもgemを自作したことがなかったり、テストはどう実行するのかも知らなかったため、覚えたかった
- コマンドラインで動かせること
- これもgemであるけれどコマンドラインで呼び出すために、何が必要かを知りたかった
- 出力結果が気になったこと
- 単純な出力でなく、以下のような出力はどうやるのか知りたかった
-
Using Movefile
の行の太い線
▬▬ Using Movefile: ./movefile.yml ▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬
▬▬ Listing Local ▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬
local: http://vhost.local
結果的に、いくつかの「どうやるんだろう?」が解消されたので、実際のプルリクエストのコードレビューの前にも、学びが多くありました。
-
logger.task(...)
を使うと太い線で出力できる - コマンドラインで利用するには、Thor を使う
- lib/ 以下にソースコードを配置している
まとめ
以上、Wordmoveにプルリクエストを出してみた、というお話でした。
あらためて、突撃的なプルリクエストでしたが、丁寧にレビューしてくださったAlessandroさんには、とても感謝しております。この体験が良かったからこそ、また何かをやりたいなと思えています。そういう意味でも、稀有な経験だったと感じています。
また、黙々と作業しつつも一緒に喜んでくれたり考えてくれる、社内版OSS Gate活動に打ち込む同僚の皆さんにも感謝を。
わたしが紹介した頃はバージョン1系で、いまや5系になっているので、紹介できていない機能があれば、Qiitaに追記したいと思います。
Wordmove、とても便利なツールですので、ご利用の方はぜひStarをつけていただければ嬉しいです。
-
「OSS Gate」は、OSS開発に参加する「入り口」を提供する取り組みです。(OSS Gateのホームページ https://oss-gate.github.io/ より) ↩
-
社内OSS Gate活動、ワークショップを実施されている企業はいくつかあるようです! ↩