programming
pullrequest

プルリクエストが最速でマージされるための7つの技

わたしはOSSとして公開されているGitHubのリポジトリに貢献することがあるのですがいつも困っていることがあります。私がPRをオープンするとめっちゃコメントがついて、それを修正するのにすごく時間がかかることです。そういえば私の元同僚のピーターは生まれて初めて新しい言語で書いたPRですら一発でマージされていて驚いた覚えがあります。どうやったらプルリクエストが最速でマージされるのか?インターネットを調べても情報がなかったので、自分で調べてみることにしました。ちなみにこのポストは、私自身が書いたHow can you get your PR merged less than 10 comments?の日本語版の記事です。

PR.JPG


プルリクエストが最速でマージされるための7つの技は次の通り


  • ガイドラインに従う

  • 静的解析ツールを使う

  • リポジトリのコードスタイルをそっくりまねる

  • プルリクエストを小さくする

  • ビジビリティとコミュニケーション

  • スペルチェックツール

  • リポジトリオーナの気持ちを考える

この記事が少しでもお役に立つと幸いです。もし、ほかにもマージされ技を持っている人はコメントをくださるとうれしいです。


ガイドラインに従う

最初のステップとして、ガイドラインを勉強するのがお勧めです。言語やフレームワークによっても違うと思いますが、例えば私がコントリビュートしているDurableFunctionsのリポジトリだとC#のライブラリですが、それにはフレームワークデザインのガイドラインというのが存在します。

ここには、ネーミングルール、型、メンバー、機能拡張、例外のデザイン、デザインパターンなど読んでいるだけで勉強になります。これはC#の例ですが、他の言語でも似たようなのがあるので是非探してみてください。。このフレームワークは古いのですが、それでも今な価値があって、シニアプログラマだったらみんな読んでいて、こういう風にコードが書かれていることを皆期待しています。


静的解析ツールを使う

静的解析ツールは常にONにしておくのをお勧めします。コーディングのスタイルや、よい書き方を教えてくれるのでとても勉強になります。私の同僚のリカルドは、静的解析ツールに指摘されたときは、ちょっと時間をつくってなぜ指摘をされたのかを理解するようにしていると言っていました。C# だったら次のようなツールが該当します。ちなみにReSharper がインタラクティブにデコンパイルもしてくれるので大好きです。


リポジトリのコードスタイルをそっくりまねる

上記の2つは基本編で、今回の記事を書くためにいろんな技術イケメンさんに話をきいてみました。その中で、ほぼすべての技術イケメンの皆さんが指摘していたのがこの点です。PRにコメントが付く数を最小にしたかったら、自分が貢献したコードの周りのコードのスタイルを踏襲していることが最重要とのことです。ある技術イケメンの方は、次のようなことを言っていました。


昔は自分のスタイルでコードを書いていたが、自分のスタイルで書きたいのをぐっとこらえて、リポジトリオーナーになり切る


このリポジトリのローカルのコードスタイルだけど、最新のプログラミングスタイルになってるとも限らないし、一般的なコーディングスタイルであるとも限りません。一度びっくりしたのが、PRをオープンしたら、リポジトリのオーナーから「privateを使わないでください」と言われて相当びっくりしました。一般的でも、自分のスタイルでもなく、「そのリポジトリのスタイルをまねること」が重要。自分のコードのほうが数段カッコいいぜ!と思っても、コードをシェアしている以上、そのリポジトリの中での一貫性が重要で、あなたが書いたコードが、明確にあなたが書いたとわかる色がでていたら、たぶん怪しいサインです。


プルリクエストを小さくする

リポジトリのオーナーになったことを想像してみましょう。毎日のように沢山issue や PR がやってきて仕訳をするだけでも大変です。もしあなたが大きなプルリクエストを送ったとして、それが、めっちゃくちゃ魅力的な機能やバグフィックスだったら頑張ってレビューしてくれるかもしれませんが、そうでない場合、レビューするだけでも大変です。そして、それだけ大きなプルリクエストだったら本当にちゃんと動くかを把握するのはとても大変です。すべての機能を完璧に実装しきってプルリクエストを送る必要はありません。大きなプルリクエストを送るぐらいだったら、複数の1つの目的をもったプルリクエストにわけると、レビューするほうも楽ですし、マージのリスクも低くなります。最悪なにか起こっても、小さければ、リポジトリオーナーが何とか出来ると思えるでしょう。


ビジビリティとコミュニケーション

リポジトリオーナーやコントリビュータ間のコミュニケーションとか信頼関係が重要で、こいつイケてる!と思っている人が何かのPRを送ってきて、小さいPRだったら、コミッタじゃないけど、その人なら大丈夫やろ!と思ってマージしちゃうのが心情でしょう。そのためには、普段から貢献してたりIssue をさばくのを手伝ってくれたり、Issue や PR でユーザからの質問に答えてくれている人をみたらそうしちゃうと言っているオーナーの方もいました。


スペルチェックツール

これはガチで日本人用のティップスですが、私が正直一番困るのがコメントの英語を考えるパートです。自分ではスタイルをまねているつもりでも、ネイティブスピーカーから見たら気持ちわるいとか当然ありますし、わかりっこありません。だからスペルチェックツールに頼るのがいいでしょう。私は、Grammarly というサービスを使っています。


リポジトリオーナの気持ちを考える

リポジトリオーナーは大変です。沢山のPRや、Issueに対応しなければなりません。彼らと私とは、機能やバグフィックスに対する優先順位もちがうので、常にマージされるとも限りません。だから、リポジトリオーナーの立場を想像して、彼らが楽になるようにしましょう。具体的には、コントリビュータガイドがあったらちゃんとそれを読んで従う、ユニットテスト、インテグレーションテスト、E2Eテストがあったらちゃんとテストを追加して、CIで安全性を確かめられるようにする。私が見た中で最も優秀なプログラマのピーターは、PRの時に、rebase でコミットをまとめると言っていました。そうでないと、fix typoみたいなリポジトリオーナにとってどうでもいいようなコミットがたくさん並ぶことになります。また、PRの最初のコメントも、PRの中身と目的、どういうことをやったかを書いてあげたり、関連する Issue, PR のリンクをつけることで、リポジトリのオーナーが全体像を把握しやすくなって、理解が深まります。

 他の作戦としては、PRを書き始める前に、最初は作りたいPRに対応するIssueを作成して、先にリポジトリのオーナーと会話をしておくと、彼らの期待通りのものを作ることができます。PRをするために、コードベースを高度に理解する必要があるときは、先にちょっとだけ実装して、PRをいきなりおくって、そこで、オーナーと会話をして仕様を固めるという方法も効果的です。最後に英語ですが、この記事を読むと、オープンソースのリポジトリのオーナーがどういう風に考えているのかがわかって興味深いです。

そこで書かれていた彼のプライオリティを引用すると



  1. メジャーバグ。現在の使われ方が阻害されてしまう何か。

  2. バグフィックス

  3. コードを読みやすくするための最適化とか、パフォーマンスの改善

  4. 多くの人々に有用な新機能

  5. その他 (あまり使われそうな機能、書き直し)


PR送る側とすると、一番楽しい 4. の優先順位が想像よりずっと低いですね。


まとめ

7つの技を紹介してきましたが、常に最速でマージをされるためには、経験やリポジトリオーナの期待を理解することが重要です。もし、PRをつくってたくさんのコメントをもらったとしても心配ありません。私の中で最もイケてるクラスのプログラマに相談したときに教えてくれたのですが、彼でも、最近60個のコメントをもらったり、あるPRを全面的に書き直したりとかしたらしいです。ただ、大抵の場合はコメント10個以下で速攻でマージされるとのことです。(彼は.NET を作っています)そんなレベルの人でも起こりうることなので心配ありません。ただ、何らかの技があって通常はものすごく少ないコメント数で効率よくPRを書いているのだと思います。もし、ほかにもいい技があったら是非コメントで教えてくださいね。


Special Thanks

この記事は私の経験というより、私が知りたかったので、いろんな人に聞いたり、教えてくれたことをまとめただけにすぎません。特に @cgillum@peterhuene はとってもいいアドバイスを沢山教えてくれました。 @xin9le, @sato_ryu, そして @mumoshu さんは、考えてもなかったアイデアを教えてくれました。 @ricardoserradas, @yu_ka1984, @shibayan, そして @yoichiro さんからは、よいコメントをいただきました! @azurekanio はPRのコツを私におしえてくれました。みなさん本当にありがとうございました。上記の名前は Twitter のハンドル名です。皆さんガチの技術イケメンです!