Git（GitHub）の運用で気をつけていること

ある日、

PR の内容を見ずにマージすることを岡島（ピッチャーの）というらしい　笑った
— いのうえ (@a_know) 2015, 9月 10

ということで、脳天気に笑っていたら、

@a_know むしろイキナリmasterリポジトリに直接pushするパターンですね！
— そーだい＠初代ALF (@soudai1025) 2015, 9月 10

という話になり、そしてなぜだか、

@a_know push -fと同様、Gitの運用アンチパターンとかどこかに纏めがほしいですねー。　#ブログ待ってます
— そーだい＠初代ALF (@soudai1025) 2015, 9月 10

というはなしになったので、~~本当に必要として頂いているのかどうかはともかく、~~ Git / GitHub でぼくやぼくの職場で気をつけていそうなことをまとめてみる。

もくじ
GitHub Flow に沿って開発する
基本的に全ての変更をプルリクエストにする
プルリクエストはすぐ作る・こまめに更新（push）する
GitHub の外で起きたことも、極力 GitHub に残すようにする
ひとつのプルリクエストはできるだけ小さくする
基本的にセルフマージはしない
push -f は別に禁止はしていない
無理して英語で書くことで意図が伝わらない・伝わりにくいなら、日本語でおｋ
まとめ
関連エントリ

GitHub Flow に沿って開発する

まずはこれかなと。

ここにもまとまっているけど、

master ブランチは常にデプロイ（リリース）可能な状態にする
何か機能を追加したり、バグを修正したりするとき（つまり、master に対して何らかの変更を加えたいとき）は、必ずブランチを切って対応する
master ブランチに変更がマージされたら、即デプロイを行う（もしくは、自動でデプロイが行われるようにしておく）

といったあたりは、基本的なこととして遵守している。

前職では Git Flow での開発だったけど、しち面倒くさいわりに何か恩恵があったかというとそうでもない気がしていたので、今のこの開発スタイルは個人的に気に入っている。もっと大規模な組織・開発になったら、また話が違ってくるのかもしれない。

基本的に全ての変更をプルリクエストにする

先ほどの参考サイトでは フィードバックや助言が欲しい時、ブランチをマージしてもよいと思ったときは、プルリクエストを作成する とあったけど、ぼくのまわりではそれに限らず、常にプルリクエストを作成してる。たとえそれが趣味のぼっち開発であっても。(´；ω；｀)

こちらがフィードバックを求めているわけではないときでも、他者からプラスアルファの助言をもらえることがある
プルリクエストにすることを意識することで、自分の中で作業内容を整理ができたりする
- 例えば、他人が見てもその目的や実現方法がある程度わかるように description や title を工夫する、とか。README 駆動開発、みたいなものに通じるものもあるかも
過去の作業を作業単位で参照・把握しやすい

といった効能があるように思っている。

特に description については、↓のような書式をすることが多い。手を抜くこともある。。

## やりたいこと
* xxx できるようにしたい

## やったこと
* xxx モデルに xxx という検索用のメソッドを作った
* XxxController を追加して、そこから検索できるようにした

## やらなかったこと
* xxx

## 確認方法
1. xxx
2. xxx
3. xxx

## その他
* 挙動的には「確認方法」で確認できますが、コード的におかしいところがないかどうかも見て欲しいです。

## マージ後TODO
* [ ] xxx

プルリクエストはすぐ作る・こまめに更新（push）する

そのブランチでやりたいことをひと通りやりきってからブランチを push・プルリクエストを作成するのではなく、最初のコミットをしたタイミングで即 push しプルリクエストを作成するようにしている。人によっては、空コミットをわざわざ作ってプルリクエストを作成している人もいる。そして、コミットを積み増す度に push 。

そういう、まだやりきれていないプルリクエストのタイトルには、それが作業中のものであることがわかるよう、 [WIP] （Work In Progress、作業中、という意味）を付ける。はやめに人の目に触れさせることで、フィードバックを受けられるチャンスを最大化する、というのが第一の目的だと理解してる。

GitHub の外で起きたことも、極力 GitHub に残すようにする

たとえば↓こんなかんじ。

f:id:a-know:20150913172022p:plain

チームメンバー同士で直接話したりして解決したことも、そのことをちゃんとコメントで残す。これも、より多くの人からのフィードバックを得る可能性を高めるための工夫のひとつ、だと思う。

ひとつのプルリクエストはできるだけ小さくする

作業をしていて、あれやこれやの "粗" を見つけると、どうしてもその場で対応してしまいたくなるけど、それをぐっと我慢する。ひとつのプルリクエストで、ひとつのフィーチャーにフォーカスするように心がける。我慢した作業（見つけた "粗"）は忘れてしまわないよう、付箋に書いてカンバンに貼り付けておいたり、issue に登録したりして、忘れても大丈夫なようにする。（人は忘れる生き物である。）

どうしても、その作業を進める上で必要なものの場合は、

そのブランチ（ A とする）から、"その粗を直すための作業をする目的のブランチ（ B とする）" をさらに切る
B ブランチも同様にプルリクエストにする
ただし、マージ先は A ブランチ

というような手順を踏んで作業を行う。"プルリクエストに対するプルリクエスト" を作るイメージ。

基本的にセルフマージはしない

さきほどのページには 他の誰かがレビューをして機能にOKを出してくれたら、あなたはコードをmasterへマージすることができる とあるけど、ぼくのまわりでは、基本的にセルフマージをしないようにしよう、ということで認識を合わせている（もちろん緊急時等は除く）。一人以上の LGTM（Looks Good To Me）を得た上で、自分以外の人にマージをしてもらう。

これの効能についてはあまり考えたことがなかったけど、レビューを促進する仕組みのひとつになっているのかもなー、と思った。今思った。

`push -f` は別に禁止はしていない

rebase したりすると push -f する必要が出てくる。push -f するのはこわいから、rebase じゃなくて merge するようにしようか、みたいな話になったこともあったけど、結局のところ、特に push -f は禁止したりはしてない。

チーム開発をしていてこれが一番必要となる可能性があるのは、やはり「自分がブランチを切って作業をしていたら」「他者の作業によって master に更新が加わり、それによって自分の作業内容とコンフリクトし、merge できなくなった」というケースかなと思うんだけど、

それがコードレビュー前なら rebase master push -f
コードレビュー後であとはもうマージするだけなら、 merge

するようにしようか、というぐらいかな。...いや、ぼくは今はもう基本的に前者かな。新人さんとかで push -f がまだこわい、という人がいて、その人とかは↑みたいなかんじかも。（話がずれるけど、それが怖いのは、単純にまだそれが理解しきれていないから、なんだよね。怖いから、といって忌避するよりも、理解して使えるように持っていきたいなとおもう）

push -f がそんなに怖いなら、GitHub 固有（GitLab にもある？）の機能だけど、最近リリースされた Protected Branch を master ブランチに対して有効にしておけばいいかも。

About protected branches - User Documentation

指定したブランチへの push -f や CI をパスしないままのマージを禁止できる。ぼくのチームの master ブランチにもこれを指定した。

2015/09/14 追記

id:koseki WIP で PR 出して、rebase & push -f ってどうなんだろ。コメントで参照したコミットが消える可能性があるって抵抗あるなー。／「レビュー前なら」まあいいのかな。誰かが自分のブランチにマージしてる可能性も気になる。

レビューコメントがつく前ならrebaseはする。コメントついたらrebaseすると米消えるのでrebaseはしない派 / Git（GitHub）の運用で気をつけていること - えいのうにっき http://t.co/i0RCtRVAUu
— Takafumi Yoshida (@zephiransas) 2015, 9月 14

ここらへん、忘れてた。

まず『自分のブランチに誰かがマージしてる場合』だけど、これもチーム規模によってアプローチを変えたほうがいいとは思うけど、ぼくのチームでは、他者のブランチにマージを行った人が「 pull してくださいー」というようなコメントをプルリクエストに残すべきだよね、というかんじの雰囲気がある。

そして、『消えてしまうのがしのばれるコメント』については、各コミットについてのコメントは行わず、file changed タブでコメントをするか、プルリクエストに対してコメントを残すように心がけている。そうしていれば、 rebase してもコメントは消えない、はず。（それでもたまにうっかりしちゃうこともあるけど。）

あと、コメントに特定のコミットのハッシュ（ abc1234 みたいなやつ）を付記して言及することもある、、というか、ぼくそれすごいやるんだけど、たしかにそれは rebase されちゃうとどうしようもなくなっちゃいますね。ｗ　ちょっと考えてみよう。。

無理して英語で書くことで意図が伝わらない・伝わりにくいなら、日本語でおｋ

コミットコメントとかの話。これは賛否両論かもしれないけど。

趣味プログラミングや OSS へのコントリビュートのためのものなら別だけど（後者の場合はむしろ英語じゃないとあかんと思う）、ぼくの会社のような「チーム全員日本人で」「日本向けのプロダクトを開発している」のなら、その意図をチーム内に正しく伝えることを最優先にするべきだと思っているし、そう言ってる。書かないと上手くならない、というのはごもっともだけど、その練習を業務でやるのはちょっと違うんじゃないかなぁ、と。