More than 5 years have passed since last update.

GithubFlowを使う上で心がけているコミット/プルリク/レビューの考え方

Last updated at 2017-11-21Posted at 2017-09-18

GithubFlowの導入をして、実際にやっていくにあたり
チームごとにルールを決めたり、プルリクエストの方法を考えると思うのですが
チームとしてではなく、自分自身の中で守ってはいきたいと思う
個人レベルでもできる心構えを『コミット』『PullRequest』『レビューコメント』の
大きく3つでまとめてみました。

コミット

コミットコメントは丁寧に

コミットに関してはコード修正に直接結びつく部分なので
その修正に関しての概要を手短に述べるようにする。

ただし、漠然とした汎用的な言い回しになり過ぎないように注意する。
また大前提としてGitではコミットの際にコメント必須だからという理由で
「コード修正」「source modify」のような空白を避けるだけのコメントはしないこと

BAD
MD5に変更

GOOD
ユーザ情報の暗号化をMD5に変更

また、コミットするまえに「この修正はコミットコメントで説明できるだろうか？」と
疑問に思うようであれば粒度が大きすぎる可能性があるので
分けてコミットすると良い

これらを行うメリットとしては以下のようなことがある。

コメントを書く過程でコードの再チェックをするので実装漏れ・ミスに気付ける
コミットする前に変更粒度を立ち止まって考えられる
レビューする際にPRよりも細かい単位でチェックができる

コミットのコメントを「タイトル」「詳細」として分けて書く

1行目をタイトルとしてコミットの種別（追加・修正・削除）を書いて
3行目以降にコミットの詳細をかき分けるようにすると
各種ツールでのコミットログの閲覧性があがります
※githubでは3行目以降の記述は折りたたまれて表示される

下記の例では [] 内に修正の種別を示すことで
コミットの種別がひと目で分かるようにしています。

例
[MODIFY] ユーザ情報の暗号化をMD5に変更

暗号強度の見直しとして
内部ストレージに保存するユーザデータのテキストデータを
単純な転置式の暗号からMD5に変更しました。

このようにすることで単純なファイル削除の場合は [DELETE] などとつければ
細かく見なくても良いという意図が伝わりやすくなります。
また、詳細を見なくても良いという人に関しては見出しだけ見れば良いので
情報の取捨選択の精度があがります。

PullRequest

PullRequest（以下PR）を作る上での心構えです。
テンプレートで管理するパターン等もありますが、それよりも一歩手前の部分での
心がけに関して書いて行きます。

粒度に気をつける

コミットの際にも記載しましたがPullRequestの粒度には気をつける。

PullRequestで上がるファイルの数が多ければ多いほど
レビュアーに目を通してもらうファイル数が増えて
どうしてもレビュー精度が下がってしまうので
なるべく細かくすることが寛容です。

例えば機能の実装だとしても
View用のファイルと実処理のファイルは分けることができるので
できるだけ細分化する方法がないかと考えて行う。

Descriptionへの心構え

たとえタイトルだけで修正箇所が簡潔にかけたとしても
書くことは色々あるはずだと思います。
ここではDescriptionの欄を書くにあたり
どんなPRのルールでも通用できそうな心構えを4つほどあげていきます。

依頼相手がPRからわかるようにする。

レビューをしてもらうためのPRなのであれば
見て欲しい相手また見てもらうべき相手に関してはPR内に記述する。
チャット等、別のシステムで依頼相手に通知する場合でも
PR内に記述しないと見て欲しい人がわかりづらくなるので記述したほうが良い。

例
アプリ開発チームの皆様、レビューよろしくお願いします。

Githubの場合は気づきやすくするためにメンションの機能を使っても良い場合は
使うと見て欲しい人に伝わりやすくなり、後で見返しやすいので良いかと思います。

例
@suzuki-taro さん、また、サービスエンジニアの皆様
コードレビューよろしくお願いします。

なお、メンションの運用に関してはアラート的な側面もある場合があるので
チームで相談するほうがベターです。

敬意を払う、しかしお堅い感じなりすぎないように

お互いレビューする側される側の関係性でお互い様であるという側面はありますが
実際問題として自分の業務の時間を割いてチェックをしてもらうので
依頼をするということを忘れずに文章を書くと良いです。
ただし、文章が硬くなりすぎると、その後のレビューでのやりとりも硬くなってしまい指摘に威圧感が出てしまうので、文体はなるべく柔らかくするとベターです。

変更箇所を書く

変更箇所を箇条書きで書くと
コミットコメントでの部分とかぶるところがあるのですが
下記のようなメリットがあります。

修正ポイントがコードを読まずとも簡潔に伝わる
コミットの粒度が適正だったか振り返ることができる
PR作成者が文章をまとめる上でコードの変更を見直すので、コミット漏れのチェックなどにも役にたつ
PRがログとして残るので、あとから振り返る際に検索性が増す

思いを書く

変更箇所に加えて、どんなことをしたかについて
PRを出す場合なんらかしら文字で記述すると思います。
そこにプラスアルファで思いを載せる良いと思います。

思いとは例を上げると以下の感じです。

コード的に微妙なんだけど諸事情により見送ったことに関しての説明
処理としてイマイチ自信がない部分の注釈
コードには書き表せなかった、参考元にしたコードや参考にした考え方
コードレビューするにあたり、頭に入れておくと理解が早まる背景事情等

思いを書く理由とメリットは以下の通り

レビューする側に前提条件を伝えることでレビューしなくてもいい部分の切り分けを明確化できる
- 「イケていない実装だが影響範囲が多いので未着手」みたいに書かれていればそこに関してのコメントは控える、等
見返したときに仕様からは掘り起こせないような事情もすぐにわかる
- 「外部からの指摘による取り急ぎ対応」のような当時の事情がわかると、別の人が参考にしていいコードか否かの判断ができる
- 単純に自分が過去を振り返る点でも有用、半年前の自分のコードは他人のコード
思いに反している部分があればそこも込みで確認してもらえる。

レビューコメント

言葉選びを気をつける

テキストコミュニケーションというものは往々にして感情が欠落しがちですし
レビューの議論が白熱すると殺伐としてしまうときがあるので
言葉使いに関しては気を使っています。

こちらのスライドやこちらのサイトにまとまっているので
そちらを御覧いただければと思うのですが
その中でも気をつけているのは

フラットな文章は感情がない状態なので、得てして不機嫌な文章に捉えられがちなので大げさなぐらい感情表現を豊かに書く。
レビューというものが指摘の作業であり、負の感情を誘引しやすいので、語尾には気をつける。

上記の2点です。以下に（極端ですが）例を挙げます。

BAD
コメントに関して修正をしてください。

GOOD
コメントがわかりづらいので、もう少し詳しく記述をお願いできればと思います！

わかりやすく、納得のできる説明をする。

レビューで指摘のコメントを入れるということは
レビュアーが何かしら悪いと感じたから指摘するのだと思います。
そのため感じた悪い部分に関してはレビュイーが納得行く形で説明しないと
レビュイーは『よくわからないけど指摘されたから直そう』となってしまい
また同じ指摘を受けるような記述をしてしまう可能性があります。

そのため『具体的な修正箇所』や『修正の根拠や出典』などを出来る限り
書き添えてあげると納得感が増します。

以下に例を示します。

BAD
この変数名は良くないので修正してください。

GOOD
変数名の customerName ですが
Rubyの場合はコーディング規約にもあるように
変数名はスネークケースのほうがよいので
customer_name に変更してもらればと思います。

コメントの種別を明らかにする

コードレビューというのはコードの修正指摘が主ですが
場合によってはレビュアーの力量不足で読み取れないコードや
修正指摘だとしてもいろいろな選択肢が伴うコードもあると思います。

その場合、どちらかわかるように最初に明記すると
コメントの反応もしやすくなります。

例
質問なのですが
unless can_read というのは
if !can_read と同等の意味になるのでしょうか？

文章として先んじて書くのもよいですが
記号を交えて表題のように書いてあげると、普段と違うコメントというのが
わかりやすくなります。

例2
［提案］
a = a.nil? ? 3 : a ですが a ||= 3 ともかけるので
変更したほうがすっきりしますが、いかがでしょうか？

まとめ

自分が日頃やっていることを言語化したく起こしたものでしたが
全体を通して言えるのは

見てもらう人にやさしく
あとから見た時にわかりやすいように

という2点が中心になっています。
この投稿が、皆様の日頃の開発の何か助けになれば幸いです。