イシューが却下されることがあるように、プルリクエストもマージに至らず却下されることがあります。頑張ってプルリクエストしてみてもなかなかマージしてもらえない場合、もしかしたら頑張り所を間違えているのかもしれません。
変更をマージしてもらえないケースでよくあるのが、コードであったり設計方針だったり運営方針だったりと行ったさまざまなレベルで、そのプロジェクトに「馴染まない」変更になってしまっているケースです。その場合、そのままの方向で頑張り続けても実を結ばず徒労に終わってしまいます。
なぜ自分の提案内容がプロジェクトに馴染まないのか、どういう所で馴染まないのか。本章では、それらをよく分析して見つめ直してみるための勘所をいくつかご紹介します。
前述した通り、筆者の考えるプルリクエストの理想は*「元のコードに馴染む、まるで元の開発者が書いたかのように見えるコード」*です。
第N章「プルリクエストしてみたい!」では、「技術力に差がありすぎると変更箇所が悪目立ちする」と述べました。しかし、技術力があっても変更箇所が悪目立ちすることはあります。筆者の経験上は、*「そもそも馴染ませようという気があるか、ないか」*という所での差異が大きいような印象があります。
詳しくは後でまた述べますが、元のコードに馴染まない変更は、バグを生み出す温床です。プロジェクトのオーナーは、ただ意地悪や気分で「馴染まない変更」を拒絶するのではなく、プロジェクトにとって現実的なデメリットがあるから拒絶しているのです。
馴染む変更をするためには、「書けばいい、動けばいい」というレベルよりも一段階上の意識が必要です。他の箇所に馴染むような変更をするためには、「他の箇所」がどういう書かれ方をしているかを把握していないといけません。つまり、変更対象の既存のコードを、変更箇所以外も含めてもう少し読み込む必要がある、ということです。
- 空白の入れ方、改行の仕方の傾向
- 変数や関数の名前の付け方の傾向
- モジュールの分割の粒度の傾向
- 全体的な設計方針、設計思想
- コメントに書かれた補足情報
などなど、既存のコードからは色々なことが読み取れます。しかし、本を読むように漫然と先頭から読んでも、内容が頭に入ってこない場合が多いでしょう。それもそのはずです、ソースコードはそのように読むようには書かれていません。ソースコードを読むには、読み方を知る必要があります1。
日本語の文章では、全く同じ内容の文章でも、どこに句読点を入れるか・漢字で書くかひらがなで書くか・「プリンター」と書くか「プリンタ」と書くか、といった「表記揺れ」があり得ます。プログラムにおいても、言語によってはそれと同様に、動作の内容自体は同じでも表記揺れが起こり得る部分があります。
たとえば、JavaScriptでは以下のような表記揺れがあります。
- 丸括弧や波括弧などの前後での改行の有無、スペースの有無。
if(experssion){
と書くのか?if ( experssion )≪改行≫{
と書くのか?if (≪改行≫experssion≪改行≫)≪改行≫{
と書くのか?
- 省略可能な要素を省略するかどうか。
- 行末のセミコロン(
;
)を必ず書くのか、書かないのか? if
での条件分岐で実行する内容が1つだけのとき、波括弧を使うのかどうか?
- 行末のセミコロン(
- インデントの仕方。
- 仮引数の書き方。
- 関数の定義時に、引数の既定の値の定義や、名前付きのパラメータとして受け取る書き方4を使うのかどうか?
「そんなの、動けばどっちでもいいじゃないか」と思いますか? だとしたら、それはあなたが、コードの表記が統一されていないことが原因で発生するトラブルを、まだ体験したことがないからでしょう。
実際には、コードの中に表記揺れが多いと、誤読や誤記によって問題が発生する原因になります。また、コードを変更する人がその都度「こっちの書き方がいい」と思った表記に変えてしまうことが繰り返されると、行の変更理由を調べても意味ある情報を得られなくなってしまいます。コードの表記揺れは、百害あって一利なしなのです。
こうした問題を避けて開発に集中できるよう、プロジェクト全体でルールに則って書き方を統一し表記揺れをなくすことを、*「コーディングスタイルを揃える」*と言います。
「外部のコントリビューターがコードの書き方まで揃えるのは大変だから、プロジェクトオーナー側で変更のマージ後に直してくれればいいのに」と思う人もいるかもしれません。実際に、そのような運用を取っているプロジェクトもあります。
しかし、プルリクエストの数が多いと、プロジェクトオーナーはそのような本質的でない作業に忙殺される羽目になってしまいます。また、リポジトリのコミット履歴も本質的でない変更ばかりになってしまいます。「プロジェクトオーナーの手が回らないところを手助けしてプロジェクトに協力する」のがコントリビューションの本質なのに、そんなふうにプロジェクトの運営を妨げてしまっては本末転倒ですよね。
前述のように機械的な判別が容易な部分のコーディングスタイルは、昨今のOSSでは、ソフトウェアで自動的に揃える運用を取っていることが多いです。prettierやESLintはそのためのソフトウェアの代表的な例ですし、VisualStudio Codeなどのテキストエディターを適切に設定していれば、ファイルの保存時に自動的にコーディングスタイルを揃えさせることもできます。
そのような運用を取っていないプロジェクトでは、コーディングスタイルを意識して合わせる必要があります。
歴史の長いプロジェクトでは、GNUのコーディング規約やMozillaのコーディング規約のように、文章の形でコーディング規約を明記している場合があります。
そういった規約が特に定められていない場合は、変更を加えた箇所について、以下のような観点で「そのプロジェクトの他のコード」を見て、他の箇所と同じ書き方になるように揃えましょう。
コーディングスタイルをソフトウェアで自動的に揃える運用をまだ取っていないプロジェクトでは、現在のコーディングスタイルを強制するルールをまとめた上で、自動化の仕組みを導入する提案をしてみてもよいでしょう。実際に第N章「他の人のフィードバックから学ぼう」では、筆者が行ったそのようなフィードバックの事例を紹介しています。
多くのOSSのコードでは、クラスや変数などの名前付けに英語の単語や熟語が使われています。同じことを言い表すのにも、考え方次第で色々な表現の仕方があり、どのような表現・表記を主に使うかはプロジェクトによって方針が異なります。特にポピュラーな表現のパターンを、以下にいくつか挙げてみましょう。
複数のデータを保持する変数の名前付けには、いくつかのパターンがあります。
- itemsなどの、データの内容を表す単語の複数形。
- itemArray、itemListなどの、データの内容+データを保持している構造を表す接尾辞。
似た例として、個数や件数などの*「数」を保持する変数やプロパティの名前付け*には、以下のようなパターンがあります。
- item_count
- item_length
- item_size
- n_items(数学や物理で「N個の」といった言い方をすることにちなむ表現)
関数名・メソッド名の動詞にもパターンがあります。たとえば、原形にするか現在形にするかは、何をさせる機能かという目的で変わることが多いです。
- String#split(分割する)などの原形:データに変更を加える、何かをさせるとき。
- String#includes(部分文字列を含むかどうか)などの三人称単数形:データの状態を問い合わせ、真偽値で結果を返すとき。
特定の文脈で使われやすい単語や表現にも、いくつかの流儀があります。状態を問い合わせる場面では、以下のような例があります。
- isItem / hasItem:Yes/Noで答える疑問文のように名付ける。
- getItemState:状態を尋ねる、という趣旨をそのまま名前に採用する。
- valid*?*:Rubyのコードでよく見られるパターンで、真偽値を返す場合の接尾辞として「?」を使う。
データの取得や保存に関わる場面でも、以下のような例があります。
- getItem / setItem:値を取得したり変更したりする物について、set-getのペアで名付ける。
- readItem / writeItem:ファイルなど何らかのデバイスやストレージに対してデータを読み書きする場合に、そのことを強調するように名付ける。
- fetchItem / downloadItem / pushItem / uploadItem :データをネットワーク越しに取得・保存する場合に、そのことを強調するように名付ける。
- save*!*:Ruby on Rails(のActiveRecord)に見られるパターンで、破壊的な操作をしたり、操作を強行したりする場合の接尾辞として「!」を使う。
また、複数の単語からなる識別子の名前付け自体にも、以下のようなパターンがあり、それぞれよく使われる場面があります。
- SomethingName:各単語の先頭1文字を大文字にする。Pascal Case5、またはUpper Camel Caseと呼ばれる。クラス名で使われることが多い。
- somethingName:2つ目以降の単語の先頭1文字を大文字にする。Camel Case6、またはLower Camel Caseと呼ばれる。
- something*_*name:単語同士をアンダースコアで連結する。Snake Case7と呼ばれる。
- something*-*name:単語同士をハイフンで連結する。ケバブケース8、チェインケース、ハイフンケースなどと呼ばれる。ファイル名で使われる。
- SOMETHING_NAME:単語をすべて大文字にして連結する。定数でよく使われる。
こういった点の名前付けの流儀を合わせることで、変更箇所のコードがより元のコードに「馴染む」ようになり、プロジェクトオーナーにとっても、よりマージしやすいプルリクエストになります。
プロジェクトによっては、こういった名前付けについてもコーディング規約で定めている場合があります。そのような明示的な規約がない場合は、やはり、変更対象の部分やその周囲の部分など、既存のコードを読んで傾向を把握する必要があります。
文章を読むときには、文法に対する理解だけでなく、単語や慣用句の知識(語彙力)も必要です。
同じことがコードを読むときにも言えます。コードの書き方には「このような動作結果を得るために、こう書く」ということ以外にも、分かりやすさ、意味の読み取りやすさなどの視点があります。「こういうことをしたい時に、こういう書き方をする」「こういう効果を狙って、こういう書き方をする」という*「コードの語彙力」があると、同じコードを読んで得られる情報の量が大きく増えます。コードを読むときの意識の解像度が上がる*、と言ったほうがよいかもしれません。ここまでで紹介したようなさまざまなコーディングスタイルを知ることで、今までは区別が付かなかった些細な違いにも、意識が向くようになるのではないでしょうか。
このほか、言語を問わず「読みやすさ」という観点にフォーカスしてコードの語彙を紹介している本としては、「リーダブルコード ―より良いコードを書くためのシンプルで実践的なテクニック(Theory in practice)」(オライリージャパンより、2012年刊行)9があります。この本を読んでからOSSのコードに触れると、「あっ、この書き方は読みやすさのための工夫をしている!」という箇所に気付きやすくなり、プロジェクト内でどのような流儀を採用しているかをより読み取りやすくなるでしょう。
また、変数や関数の名前付けといったミクロな部分だけでなく、モジュールの設計というマクロな部分にも、色々な流儀があります。設計の流儀でよく知られているのは、*「デザインパターン」*です。皆さんも「シングルトン」とか「オブザーバー」とか「イテレーター」とかいった言葉を聞いたことはないでしょうか?
デザインパターンは、複雑な問題に対処するときの典型的な設計パターンとしてよく使われています。本書では掘り下げませんが、例を交えての解説もすでに世の中に多数存在するので、概要だけでも頭に入れておくことをおすすめします。そうすると、コードを読むときに「あ、これは○○パターンだ!」と知識を当てはめられるので、見通しがよくなり、理解が進みやすくなります。
既存のコードを頭からすべて読み下すのは、規模の大きなソフトウェアでは非現実的です。筆者は、ユーザーとして自分がよく使う機能など、特定の動作に対応するコードから読むのをおすすめします。具体的には、たとえば以下のようなやり方があります。
- 警告やエラーのメッセージなど、画面に表示されるメッセージ文字列の定義からたどる方法。「このメッセージを表示している処理はどこか」ということを、ソースコード中を検索して特定する。
- 「HKEY_CURRENT_USER」など、設定のキー名やレジストリのキーに使われる文字列からたどる方法。「この文字列が埋め込まれている場所はどこか」ということを、ソースコード内を検索して特定する。
- 機能の名前からたどる方法。「この機能を実装しているモジュールや関数の定義場所はどこか」ということを、ソースコード内を検索して特定する。
- 既知のAPIからたどる方法。たとえばWindows用アプリケーションであれば、.NET Frameworkの「RegistryValueKind.QWord」という定数名でソースコード中を検索すると、Windowsレジストリを読み書きしている箇所を特定できる。
- その言語やAPIを知らなくても、「Windows レジストリ 読み書き」のようなキーワードで*「その分野でそういうアプリケーションを作りたい人向けの情報」*を探すと、調べるべきキーワードにたどり着けることがある。
知っている挙動に確実に対応している箇所をソースコード中から特定できれば、あとはその処理の呼び出し元をたどっていくことで、「なるほど、この動作を実現しているのはこんなコードなんだ」と、「既知の挙動の前後の挙動」と「呼び出し元の前後のコード」とを対応付けて読むことができます。
ソースコードを編集して独自改変版を動かせる状況であれば、その箇所で例外を意図的に発生させるように改変してみてもいいでしょう。例外発生時のスタックトレース(バックトレース)を見て、処理の呼び出し元を一気に把握することもできます。
また、コードを読む手がかりとなる資料としては、自動テストも有用です。自動テストでは多くの場合、テスト対象のモジュールをその場で初期化して各機能を呼び出して使うため、実際の動作時にどんな引数を伴ってどんな呼び出し方がなされるのかを把握する上で、大変参考になります。
コードの変更そのものとは無関係ですが、コミットログの書き方の様式も、そのプロジェクトの流儀に合わせるようにしましょう。プロジェクトによってはコミットログをリリースノート作成時の参考資料にすることがあり、あまりに様式から外れていると、リリース時に開発者に余計な苦労を強いることになってしまいます。
コミットログは基本的には、5W1H10のうちの「Why」にフォーカスをあてて書きます。たとえば以下のような要領です。
- 「ユーザーがグループに所属していないときにログインに失敗する問題を修正」
- 「チケットの作成時に空のフィールドを許容しないように変更」
このような情報が含まれないコミットメッセージは、あまり喜ばれません。たとえば以下の要領です。
- 「変数をリネームした」(何のために?)
- 「チェックを追加した」(何のチェックを、なぜ?)
- 「アルゴリズムを変更した」(どういう意味があって?)
プロジェクトによっては、リポジトリに含まれるドキュメントの修正のコミットメッセージ冒頭には「doc:
」、自動テストの修正のコミットには「test:
」、といった印(プレフィクス)を付けるよう運用で定めている場合もあります。
コードの変更に取りかかるときは、その前に、開発への参加の仕方についての説明があればそれを参照して、運用ルール自体を把握するとよいでしょう。他の変更がどんなコミットメッセージでなされているか、実際のコミットログを確認するのもおすすめです。
変更箇所のコードやコミットログを既存の物に合わせるのは、その変更を受け入れた後も、開発者が違和感なくその箇所に手を入れ続けられるようにするためです。
コードの書き方に見られる様々な流儀の例を先に紹介しましたが、OSSでのプルリクエストにおいては、「どの流儀が優れているか」ということ自体はあまり重要ではありません。それよりも、そのOSSではどの流儀に従っているのかを把握して、流儀を合わせることが大事です。なぜなら、プロジェクトの中で流儀が統一されているかどうかのほうが、コードの読みやすさや開発の効率に与える影響は大きいからです。
あなたはプルリクエストをした後は、それ以後もうそのコードのことを読み返さないかもしれません。しかし、受け入れた側のプロジェクトオーナーは、その後もずっとそのコードをメンテナンスし続けることになります。効率よく作業するためには、何かあったとき、プロジェクト内の一般的な傾向に従って探せば問題箇所をすぐ見つけられる、という状態を維持することが大切です。名前の付け方も、設計の仕方も、よく「馴染んだ」コードにするのはそのためなのです。
本書は特定のOSSプロジェクトに特化せず一般的な解説をするようにしていますが、小規模な個人開発のOSSではよくても、多人数が関わる大きなOSSとなると、そのプロジェクト固有の事情が増えてきます。一般的には問題無くともそのプロジェクトでは禁止事項になっている、という事柄がある場合もあります。
タイミングがいいことに、本書の執筆時期に前後して、大規模なOSSプロジェクトに深く関わっている人の手による、「初めてのフィードバック」や「フィードバックでつまずきやすい点」に焦点を当てた良質な記事・資料が複数公開されていました。本書と併せて読んでいただけるよう、いくつかご紹介します。
Node.jsへのコントリビュート解説、そしてOSSへ貢献するということ
Node.js Core Collaboratorで関西Node学園主宰のMasashi Hirano(shisama)氏による、Node.js Advent Calendar 2019内の記事です。「Node.jsプロジェクトではこうなっている」という文脈で、本書ではあまり詳しく述べていない、プルリクエストの具体的な手順やベストプラクティスに多くの分量が割かれていて、本書だけではまだピンと来なかった方が参照するのにうってつけの内容です。
Rubyコミッターの武者晶紀(knu)氏による、平成Ruby会議01での発表の資料です。プロジェクト運営側として活動されていることから、「こういうプルリクエストは送られても困る」と判断されやすい事例が、その理由も含めて多数紹介されています。プロジェクトオーナーの視点に馴染みが薄い人にとっては、「向こうから見ると、そういう見え方になるのか」という学びを多く得られるでしょう。
Tama.rb主宰のfuqda氏による、Tama Ruby会議01での発表です。プロジェクトの方針に合わないプルリクエストをしてしまい、識者のアドバイスを受けて方針に合うように修正してマージにこぎ着けた事例が紹介されています。自分一人では詰まってしまうような場面で、学校や会社の先輩などの近い距離に頼れる人がいない場合でも、コミュニティに関わると先達の手助けを得られる、ということがよく分かります。
プロジェクトのスコープに収まる問題として提案し、自分の目で「プロジェクトによく馴染んでいる」と思えるレベルにまで練り上げたプルリクエストも用意したとしても、要望・提案を受け入れてもらえない場合はあります。最終的に要望を受け入れるかどうかは、そのプロジェクト内部の人が決定することです。外部から協力するだけの立場の人には、残念ですがその判断への介入はできません。
そういうときに、外部協力者の立場で取れる最後の手段が、プロジェクトのフォーク11です。設定されているライセンスの条件に従う限り、誰もが自由にそのOSSを改造し、再配布することができます。プルリクエストのために一時的にフォークするのではなく、改造の成果を元の開発プロジェクトに還元しないまま独立したプロジェクトとして継続する、という選択肢です。
リソースの無駄な分散を防ぐためにも、ユーザーの混乱を避けるためにも、フォークは可能な限り避けるのが望ましいです。どうしてもフォークする場合は、それ以後の開発やメンテナンスに要するすべてのリソースを自前でまかない、最後までそのソフトウェアの面倒を見続ける覚悟が必要です。
そのため、大きなOSSプロジェクトほど、フォークは珍しいです。近年で観測された大きなフォークといえば、WebKitからGoogleがフォークして始めたBlinkのように、フォーク主体に膨大なリソースがある場合か、OpenOffice.orgからのLibreOfficeやownCloudからのNextcloudのように、フォーク元の主要開発者達が何らかの理由で元プロジェクトから離反して、フォーク先にそのまま合流した場合くらいでしょう。
Firefox 28からフォークしたPale Moonや、Firefox 56からフォークしたWaterfoxのように、個人や少人数のグループが大規模なプロジェクトをフォークしたケースは、健全な状態を保ち続けられていないことが多い印象が、筆者にはあります12。「フォークしたはいいものの、実際やってみたらメンテナンスが意外と大変で、しばらくしたら更新が滞ってしまいました」というのは、ユーザーに過剰な期待を持たせた挙げ句に振り回してしまう、最悪の結末です。
しかし、そうは言っても「絶対にフォークしてはならない」わけでもありません。
上述のとおり、プロジェクトの運営主体を完全に引き継いで責任を持って継続できそうなのであれば、あるいは、フォーク版を自分しか使わなくて不利益はすべて折り込み済みなのであれば、フォークは選択肢の1つとして考慮する価値があります。
実際に筆者にも、元プロジェクトには絶対に取り込んでもらえない独自の機能が、どうしても切実に業務上必要だという理由で、他の人の開発したOSSをフォークして以後、ニーズに応じて回収を続けながら10年近く維持し続けている実例があります。
*「いざとなったらフォークして続けられる」*のは、OSSの強みです。もしフォークという手段が許されていなければ、やりたいビジネスを諦めたり、自分の使い方での致命的な不便を強いられたりしなくてはなりません。覚悟と努力次第で困難を乗り越えられる余地があることに安心感を覚え、逆に、プロプライエタリな製品には「これ、いつまで使い続けられるんだろう?」と不安を覚える、というのは、誇張ではなく筆者の実際の思いです。
「そんなタフな選択ができるのは、あなた(筆者)がスーパーエンジニアだからだ」と思われるでしょうか?
いいえ、そんなことはありません。筆者のOSS開発者としてのキャリア自体、他の人が開発を放棄して動かなくなってしまったソフトウェアを、勝手にフォークして、再び動くように見よう見まねで修正したところから始まっています13。当時の筆者はオブジェクト指向もロクに分かっていないようなド素人でした。その元ソフトウェアはごく単純な物でしたが、それでも当時の筆者にとっては、全貌を理解しきれない、手に余る代物としか思えませんでした。
しかし、動かなくなっていた原因箇所をどうにか直して動く状態に戻せたことで、筆者はそれを足がかりとして学習を進められました。当時の筆者にとっては、入門書に書かれていた内容(基本中の基本)と自分のやりたかったこと(高度なGUIを持つソフトウェア)との間のギャップが大きすぎて途方に暮れていましたが、「すでに動く状態のソフトウェア」のソースコードを読むことによって、「やりたいことを実現できた」という喜びや、「そうか、こういう機能を作りたいときはこういう書き方をするんだ」という実践的な知識を得ていけたのでした。
また、ソフトウェアの公開の仕方、継続的なメンテナンスの仕方、要望の受け答えの仕方など、コーディングそのものとは異なる周辺領域の様々な経験についても、そうして「自分のプロジェクト」を持つようになって以降、必要に迫られて身に着ける形で、非常に多くのものを得られました。
本書は「OSSへのフィードバックの仕方」を解説していますが、実際の所、良いフィードバックとは「プロジェクトオーナーが受け取って嬉しいと思えるフィードバック」の一言に尽きます。どうだったら嬉しいのかということは、プロジェクトオーナーの立場を経験してみるとよく分かる部分が多々あります。そういう機会になるという意味で、皆さんもフォークをきっかけに「自分のプロジェクト」を持ってみてはいかがでしょうか?
Footnotes
-
既存のコードの読み方について、筆者は以前に業務で、新入社員の方向けの「既存のコードを変更するノウハウ」を学ぶ研修を主導させていただいたことがあります( https://www.clear-code.com/blog/2018/11/12.html )。本章の内容の一部は、そのときの講義内容に基づいています。 ↩
-
いわゆるタブ文字。 ↩
-
いわゆる半角スペースを何個も書く方法。 ↩
-
たとえば、JavaScriptの関数の引数での分割代入を伴った既定値の定義( https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Functions/Default_parameters )は古いバージョンのブラウザーでは動作しない。 ↩
-
Pascalというプログラミング言語でよく見られた書き方であることから。 ↩
-
ラクダのこぶのように見えることから。 ↩
-
ヘビが地を這うイメージから。 ↩
-
肉を串に刺して焼く料理「ケバブ」の様子に似ていることから。 ↩
-
「Who(誰が)」「When(いつ)」「Where(どこで)」「What(何を)」「Why(なぜ)」「How(どのように)」という、ニュース記事を書くときに盛り込むべき情報の一覧を端的に示したもの。 ↩
-
プロジェクトのリポジトリを複製したり、ソースコードだけ引き継いだりして、元々のプロジェクトとは別に、新たにプロジェクトを立ち上げること。 ↩
-
これらのフォーク版は、フォーク以後のFirefoxで行われた性能向上や安全性向上のための抜本的な変更、最新のWeb技術への対応などが反映されておらず、最新のFirefoxと比べるとだいぶ見劣りする状態になっています。 ↩
-
実を言うと、このときフォークの元にしたソフトウェア(Mozilla Application Suiteのコンテキストメニュー拡張機能)にはオープンソースライセンスが設定されていませんでした。自分の記憶が正しければ、元の作者の方が「習作のような物なので権利は主張しない」という旨のことをおっしゃっていたため、パブリックドメイン扱いとして、こちらでオープンソースなライセンスを設定した、という経緯だったように思います。 ↩