良いPullRequestとCommit Message

今回はこちらの<a href="https://gist.github.com/koudaiii/77ab8eb30512978d1122">記事</a>を参照させていただいています。

良いコミットメッセージは<a href="https://gist.github.com/mono0926/e6ffd032c384ee4c1cef5a2aa4f778d7">こちら</a>をご覧ください。

#1 小さく作る

小さくすることに集中したPull Requestは取り込まれやすい。

大きくなってしまったら、レビューアーと調整して行ったほうが良い。
レビューは5分毎に刻んで行うことが出来ず、まとまった時間が必要だからだ。
Pull requestを送るものによるが、
大体12ファイル以下の変更なら、悪く無いというべきだろう。

#2 一つのことだけをする
一つのクラスに一つの責任をもつようにPull requestもそのように一つに絞って出す必要がある。
例えば、あなたが3つの独立した機能をもつPull requestを送った場合に、
AとCは有用であり取り込むべきとなったが、Bには問題があるといったことがしばし起こる。
その場合、Bについて長い議論をし、最終的にPull requestを変更する必要な場合がある。

長い議論の間にもMasterではいくつかCommitされているから。
長い議論の間に既にAとCについては自動でマージできない状況になる。

AとCは既に完璧なものであったにも関わらず結局、宙に浮いている状態である。

そのかわりに、A,B,Cすべてを独立してPull Requestを送った場合、AとCはすぐに取り込まれるだろう。
一つのPullRequestは、一つのことに集中にして行うようにするべきである。

#3 一行の長さを考える
あなたのRullRequestをReviewをする際にレビューアーはDiff toolを使う。
GitHubもStashもWebベースのDiffツールを提供している。
レビューアーは提供されている画面分割されたDiffをみている。
それはCodeを画面半分でも読めるようになっている必要がある。

あなたがかいたCodeが横に長かった場合、横スクロールを強制してしまう。

[80文字以内にする理由は多くある。](http://richarddingwall.name/2008/05/31/is-the-80-character-line-limit-still-relevant)リンク先のブログを参照すると良い。


#4 フォーマットの変更を避ける

フォーマットを変えたい衝動に駆られるかもしれない。でもそれはやめてほしい。
フォーマットを変更することですべての差分に出てしまう。
スペースの差分を出さないオプションをあるが、それをレビューアーが無視するには限界があるんだ。
特にCodeを移動した場合、それを無視することができない。やめてほしい。
スペースの変更がしたい場合、
ファイル内の前後のCodeを移動、フォーマットの変更、スタイルの変更を別のPullrequestにしてほしい。
そしてその主旨をコメント書いてほしい。

#5 ビルドの確認をする
Pull requestする前に、ローカルでビルドの確認してほしい。
ローカルで動かないのであれば、多分他のマシンでも動かないんだ。

コンパイラの警告は注意してみてほしい。
もしかすると、あなたが気づかないうちにコンパイル警告によって、
レビューアーを妨げるかもしれない。
あまりりの多くのコンパイラの警告がある場合、あなたのPull requestを拒否するかもしれない。

もしビルド用スクリプトがあれば、試してほしい。
そしてビルドが成功したやつのみPull requestを起こって欲しい。

私の最も多くのOpenSourceのプロジェクトでは、警告をエラーとして扱うビルドスクリプトを作っている。
このようなビルドスクリプトは自動かまたはさまざまな特定なCodeにおいてルールを実装している。

Pull requestを送る前に使ってほしい。
なぜなら、あなたのブランチをマージする前にも利用するからだ。

#6 すべてのテストが通ること

テストコードが書かれている場合、
一度すべてテストを実行してほしい。

これは言われる前にするべきだ。
でも、私のところにはひとつまたはそれ以上でテストが通っていないPullrequestが届くんだ。。


#7 テストの追加
また、すでにいくつかユニットテストが書かれている場合において、
あなたのPullrequestをテストするCodeを追加しよう。

私のところにテストがないPullrequestは余り届かないが、
しばしば私は拒否する。

これはハードなルールではない。
テストのカバレッジなしの必要ないろいろな種類のケースが存在する。

でもテストができるものは、テストをするべきなんだ。
そして、あなたは、既存のコードベースのテスト戦略に従うべきなんだ。

#8 理由をドキュメント化する

ドキュメント化したCodeはまれである。
「Codeにあるコメントは謝罪である」というが、
そして、私はコメントより型や変数が良い名前を好む。

まだCodeを書いている時、
私は自明でないかどうかしばしば意思決定をしなければならない。
(特にビジネスロジックの取引を行うときなど)

「何を」書いたのかではなく「なぜ」あなたがそうCodeに書いたかを文書化する

私の望ましい順に書いていく。

1 文書化されたCode
あなたは、自己文書化されたCodeについて、意思決定することができる。
Clean Codeは、文字通りそれについてのHow-toが書かれている。

2 Codeにコメント
もし１が難しい場合、コメントを書こう。
少なくとも、Codeと同じ場所にコメントを書こう。
バージョン管理システムが変わってもコメントが消えないようにする。
ここに、私がみたコメントで対処したほうが良い例がある。
```
  internal static string[] GetSegmentsFrom(Uri href)
  {
    /* The assumption here is that the href argument is always going to
     * be a relative URL. So far at least, that's consistent with how
     * AtomEventStore works.
     * However, the Segments property only works for absolute URLs. */
    var fakeBase = new Uri("http://grean.com");
    var absoluteHref = new Uri(fakeBase, href);
    return absoluteHref.Segments;
  }




3 コミットメッセージ
ほとんどのバージョンシステムではコミットを記述する機会がある。
そして多くの人が、必要最低限の記述のみとしている。
しかし、あなたはここに理由を書くことができるんだ。

幾つか、あなたは特定の順序でコミットを行わなければならず、説明する必要があるかもしれない。
この順序については、Codeのコメントより、コミットメッセージにフィットする。
あなたが同じバージョン管理システムを利用する場合に限り、
あなたのこれらのコミットメッセージは保存される。

しかし、一度実際のCodeからファイルを削除、
または、
別のバージョン管理システムを変更した場合メッセージが失われるだろう。

ここにたくさん[Commitメッセージ](https://github.com/GreanTech/AtomEventStore/commit/615cdee2c4d675d412e6669bcc0678655376c4d1)を書く必要があった例がある。ただ、いつもそうであるとは限らない

4 Pull requestのコメント

まれに、上記の方法で適さない状況がある。
GitHubやStashを使っている場合に、Pull request自体にメッセージを追加することができる。
このメッセージは、Codeのコメント、コミットメッセージから更に離れている。
同じバージョン管理システムのホストを使用している限り保存されている。

例えばCodePlexからGitHubに移行すると、
Pull requestのメッセージが失われるだろう。

それでも、場合によってレビュアーに説明する必要があるだろう。
ただし、ソースコード外での説明が必要な場合である。
この方法が適している[例](https://github.com/GreanTech/AtomEventStore/pull/70)が以下である。

あなたは明らかなものについては説明する必要がない。
しかし、慎重に考えてほしい。

今日のあなたが明らかだとしても、他人が明らかだと限りらない。
まして、三ヶ月後の自分が明らかであるとは限らないんだ。

#9 上手く書く

良いコードを書くだけでなく、良い文を書こう。
これは主観的である。しかし、文とCodeの両方にルールがある。
Codeには正しいルールがある。そうでなければコンパイルはできない。
(インタープリタの場合、落ちるんだ。)

それはPull requestのメッセージ、コミットのメッセージ、Codeのコメントも同じなんだ。

正しいスペル、文法、句読点を使ってほしい。
そうじゃなかったらあなたの文を理解するのは難しい。

そしてあなたのレビュアーも人間なんだ。


#10 スラッシングを避ける

レビュアーはPull requestについて、様々な点において指摘をする。
そしてあなたはそれに対して同意するだろう。

これは、Pull Requestのブランチに多くのコメントを残す原因となるだろう。
それ自体には何も問題はありません。

しかしながら、これは不当なスラッシングにつながる。

例えば、
あなたがA,B,C,D,Eの5つのコミットをしたPull requestをして、
レビュアーがBとCのコミットが好きじゃない場合に、Codeを消してと要求してきた。

ほとんどの人はPull requestのブランチをチェックアウトしている。
そして、問題のあるCodeを削除している。
つまり、A,B,C,Dと続き別のコミットFをしている。

なぜ我々は、不要なCodeを追加して、また削除するコミットを追加するのだろうか？

これがスラッシングである。
つまり、無意味なcommitは何も提供しない価値である。

代わりに、該当のコミットを削除して、
強制的に、A,D,EのコミットをPushするようにしよう。
レビュー中のブランチのオーナーはあなた自身なんだ。

他のスラッシングの例として、
古くなってきたPull requestに起きる。(長い議論で起こる)
この場合、Pull requestのブランチのオーナーが定期的にPull requestが通る用にMasterからマージして最新を保つんだ。

もう一度、なぜ私は、すべてのMasterのコミットのMargeの見なければならないのか？
あなたはPull requestのオーナーである。
Pull requestをrebaseして、強制的にPushしてほしい。
そうすれば過去のコミットはすべてキレイになるんだ。


#まとめ
あなたのPull requestは一人または多くの人がレビューする。
レビュアーに仕事をさせてはいけない。
あなたがレビュアーにより仕事をさせた場合、あなたのPull requestはリスクであり拒否されるだろう。

原文
http://blog.ploeh.dk/2015/01/15/10-tips-for-better-pull-requests/
日本語版
http://yakst.com/ja/posts/1625

#Commit Messagesの表現傾向とまとめ

単語	|出現回数
--------|-----------
Add	|1149
Fix	|1014
Update	|584
Remove	|566
Use	|382
Don't	|260
Make	|228
Move	|178
Change	|103
Rename	|85
Improve	|76
Avoid	|68
Allow	|65
Implement|	60
Handle	|58