Githubのレビューフローを正しく使おう、或いは「LGTM」の再考

## はじめに

Githubでのソースコード管理・コードレビューはあらゆる開発の現場でデファクトスタンダードになっているが、その詳細、つまり**「どう管理し、どうレビューするか」**は各担当者の考えに依拠する。

宗教論争は無意味なので避けたいが、少なくとも自分なりの解は明示できるようにしておきたいので、パブリックに文書化することをこの記事の目的とする。

なので、タイトルの「Githubのレビューフローを**正しく**使おう」は精確ではない。厳密に言えば**「どのような機能があるか認識していて、信念を持って取捨選択しよう」**ということである。

## TL; DR

1. 2016年9月にリリースされたGithubのレビューフロー
1. 1.に付随するmergeablity protection
1. 「LGTM」の再考
1. 管理者が管理するべき3つのこと
1. おまけ：実はリリースされていたGithubのその他の機能

## 2016年9月にリリースされたGithubのレビューフロー

ソースコードを俯瞰して各行にまたはPull Request全体にコメントを残すという従来の仕様から、

* 各行にコメントを残しかつそのコメントに対するレビュイーからの **フィードバックが明示的に分類できる** ようになった
* **プルリク全体への総評を「許可」「却下」として明示** できるようになった

ことが最大の特徴である。

> "Files Changed" タブ右上の "Review changes" から、プルリク全体への総評を提示する。
* ただコメントしただけの場合: Comment
* マージを許可する場合: Approved
* 変更を要求する場合: Request changes
![Screen Shot 2016-10-22 at 2.43.11 PM.png](https://qiita-image-store.s3.amazonaws.com/0/105252/9ac55086-5f0a-4b39-0135-e5c1633827f9.png)

## ↑に付随するmergeablity protection

明示的に `Approved` なのか `Request changes` なのかを提示できるようになったので、 `Approved` になるまでprotected branchへのマージができないという仕様も追加された。素晴らしい。

> Seetings => Branches セクション内でプロテクト対象のブランチを設定していることが前提。
![Screen-Shot-2016-10-01-at-12.37.05-PM.jpg](https://qiita-image-store.s3.amazonaws.com/0/105252/35a96883-c405-9f33-36ab-e064bf3f07a9.jpeg)

---

> Protected Branchに設定している場合、Approvedされていない場合のマージの禁止と、ついでにプルリクテストが完了していないとマージできない設定が行える。
![Screen-Shot-2016-10-01-at-12.37.48-PM.jpg](https://qiita-image-store.s3.amazonaws.com/0/105252/2d99ff37-2cda-44fb-328e-6cd965731758.jpeg)

---

> この状態で当該ブランチにPRが出されると、Approvedされていないからマージできないよ、という状態になる。
![Screen Shot 2016-10-18 at 9.50.29 AM.png](https://qiita-image-store.s3.amazonaws.com/0/105252/dc0ce5a1-4f31-74c8-986c-216377e3d0b7.png)

---

> Request Changesが設定されると、その旨が明示される。
<img width="792" alt="Screen Shot 2016-10-17 at 4.47.06 PM.png" src="https://qiita-image-store.s3.amazonaws.com/0/105252/a5385254-1394-87db-a394-6ac74bf6b697.png">

## 「LGTM」の再考

Github側で明確に **Approve** という言葉を採用したことには注意したい。普段見かける「LGTM (Looks Good To Me)」や「いいと思います！」という言葉には、「マージを許可する」「不具合が起きないことを確認した」という意味は含まれていない。

本来「LGTM」は「（デザインやクリエイティブなど、客観的に良し悪しが判断できないものに対して）私はいいと思うよ（でも、クライアントや消費者がどう思うかはわからない）」のように、あくまでも個人的な感想を伝えるために生み出された言葉である。ソースコードやプログラムも、人によって哲学は違うだろうけれど、開発における「運用プロダクトへの結合許可」は、「いいと思う」ではなく **私はそれを許可したので、問題が生じたら全面的に責任を分け合おう** という意思表示でなければならない（はず）。

だから、 **Approve** という単語に毎日触れることで、お互いに責任を持てるような意識が高まるのではないかと、密かに思ってはいる。

## 管理者が管理するべき3つのこと

上記に加えて、自分がGithubのレポジトリ管理者になったら、以下の３つを実践してほしい。

* Collaboratorの管理 => 当たり前のことである。新しいメンバを追加したり、退職・異動したメンバを削除したりする。
* READMEの監査 => レポジトリの目的や運用方法・ディレクトリ構造などのドキュメントとなる `README.md` がちゃんと更新されており、内容が誤っていないことを、定期的に監査する。
* チームに不要な機能を非表示に => 例えば、 `Settings => Options` タブ内で `Wikis` や `Issues` の必要性をアンチェックできる。使わない機能は、最初から表示しないことで、１秒でも開発者の時間を効率化しよう。
![Screen-Shot-2016-10-01-at-12.36.37-PM.jpg](https://qiita-image-store.s3.amazonaws.com/0/105252/7ca41352-66a4-e870-5f10-941d28eb119e.jpeg)

## おまけ：実はリリースされていたGithubのその他の機能

1. マージしたらそのまま `Delete branch` できるようになっていた。
![Screen-Shot-2016-10-21-at-6.40.03-PM.jpg](https://qiita-image-store.s3.amazonaws.com/0/105252/87f7aea6-8b54-2ea5-2395-a89915892032.jpeg)

1. マージ先ブランチとコンフリクトが発生していたら、プルリク上でそれがわかるようになっていた。でも、どうせgitのコマンド上で解決するから、要らないといえば要らないかもしれない。
![Screen Shot 2016-10-11 at 12.33.39 PM.png](https://qiita-image-store.s3.amazonaws.com/0/105252/3c072ea9-3ca1-b606-c955-fac5c49a5c7f.png)

1. プルリクをSubmitした後でも、マージ先ブランチを変更できるようになっていた。プルリクタイトルを変更する `Edit` ボタンを押した後、密かに `base` の変更ができる。結構便利。
![Screen-Shot-2016-10-22-at-2.32.51-PM.jpg](https://qiita-image-store.s3.amazonaws.com/0/105252/7119ba0e-ce9c-ee90-9e97-decd8b437651.jpeg)