TechとPoemeの間

Qiita に書かないエンジニア業の話

Git / GitHub 初級者をメンタリングするときに考えていること

Git や GitHub を全く触ったことがない人や，使ったことはあるけど複数人でのチーム開発で読みやすさやレビューしやすさを意識した運用をしたことがないエンジニアや，エンジニアの卵のメンターになることが多いので，自分なりのポリシーをメモ程度に列挙してみる．
もちろん，プロジェクトでポリシーが決まっていればそれに従うべきだが，自分が今いるチームや，これまで所属してきた組織では「なんとなく」でしか決まってないので，そんなチームでメンタリングするときは，「どんな現場に行ってもある程度はしっかりできる素地」みたいなのを意識している．
半分くらい http://qiita.com/meganemura/items/8a034e2065b299135c87 の焼き直しになってしまっているけど，気にしない．

ポリシーに「必須度」を付けているが，おおよそ以下のような感じ．メンタリングする時は，まず必須度 A が出来るように，少し口うるさくでも言う．本人が，自ずと他のエンジニアとのコラボレーションを気にかけるようになったら，B や C の内容をヒントとして与えていくイメージ．

  • 必須度 A : 必ず守る
  • 必須度 B : 難しいケースもあるけど，守れるなら守る
  • 必須度C : やってあると嬉しい

コミットメッセージのフォーマット 編

1行目は，行った変更のサマリーを 100 文字以内で書く．(必須度: A)

100 文字というのはおおよそだけど， GitHub 上で 120 文字？あたりで折り返されちゃうみたいなので，長くならないように意識している．
PR 画面のコミット一覧で見栄えがすればよれでよし．

2行目は空行，3行目以降に必要であれば詳細を書く (必須度: C)

2行目空行は異論ないと思う．
ちゃんとしたプロジェクトだと，3行目以降に書くことのフォーマットが決まってたりすると思うけど，ここはメンバーの善意に任せている感じです．
でも，個人的には，例えば不具合の改修で，修正した場所と，実際に起きた不具合の根本原因の箇所が遠い場合などは，後からコミットログを読んだときにどういう問題に対処する修正なのかが分かるように，意図を説明するようにメッセージを書いたりしています．

内容のないコミットメッセージを書かない (必須度: A)

「コンフリクトを解消」とか「修正」とかっていうコミットログをやめましょう，という話です．初歩的だけど，技術ではなく姿勢の問題なので，1行の変更でも，何が問題で，何が間違っていたので修正したかを説明するようにします．
コードレビュー後の「指摘事項修正」も，レビューを受けてどういう問題を直したのか，というのが分かるようにしたいですね．

コンフリクトの解消に関しては，「それを言うのは初心者には酷じゃ…??」と思う方が多いと思います．自分もそう思ってて，他のエンジニアの作業と大きくバッティングするようなタスクを渡さないとか，大きすぎて長く時間のかかるようなタスクはそもそも振らない，というメンターの問題だと考えています．メンティーの状況に応じて， rebase を教えるチャンス，とも捉えることも出来ると思います．

句点やピリオドは使わず，英語なら命令形，日本語なら「xx した」と書く (必須度: A)

自分のコミットログを見返すと，守ってないケースもあるけど…
英語のコミットログだと，「主語を使わず現在形の動詞から始める」「ピリオドで終わらない」というのがよくある規約です．英語の場合はこれを守るのですが，
日本語の場合だと，やはり主語は使わず，「xxx をした」で終えるようにしています．これは「xx する」でも「xx しました」でも良いと思いますが，プロジェクト内で統一されているべきですね．

ソースコード生成のコマンドがある場合はコミットメッセージに含める (必須度: C)

これはできればで良いのですが，例えば Rails の bundle install ... や rails generate scaffold ... は，可能であればコミットログに含められると，後からそのコマンド自体にも問題は無かったのかなども振り返ることができます．

コミットの作り方

自動生成や Initial Commit 以外で巨大コミットを作らない (必須度: A)

どれくらい，という指標はないのだけど，「ついでに xx も直していっしょにこのコミットに入れました！」みたいなことをやっているのを見つけたときは，早めに指摘をするようにしています．

どのコミットタイミングでも，ビルドやテストはすべて通るようにする (必須度: B)

可能であれば，ですが．

アプリケーションやシステムの挙動を変えるコミットと，挙動を変えないコミットを分ける (必須度: B)

リファクタや不要な変数や文の削除などは，それだけでコミットを作るようにします．レビューの対象になる主変更はそれだけでコミットをするようにすると，レビューがし易いですね．
例えば，新しい API の追加をする際に，既存のコードやテストコードをリファクタして拡張性を担保して，新しい API を追加するという手順を取る場合，リファクタのコミットと API の追加のコミットを別にします．
このように作業するように心がけると，未使用変数の削除などのコミットは眺めるのに2秒もかけないで済むし，最初のリファクタに問題点はないのかの観点でもレビューされやすいし，追加した API が妥当かのレビューに注力し易いです．

新しく追加した class や API に対するテストは，同じコミットにする (必須度: B)

例えば 「xxx クラスを追加した」というコミットの後に 「xxx クラスのテストを追加した」というコミットをつけるのではなく，この2つは一つのコミットにすべきです．
ひとつのコミット内容を見たときに，追加した API やクラスと，その使い方を示したり正しさを保証しているコードは一緒になっているべきだと考えるからです．

ブランチ / PR 運用

PR は小さく．

これは，「どれくらい」というのが言えないので必須度が付けられないのですが，やはりなるべく1つの PR は小さくある方がレビューしやすいです．
利用している言語によっても違うのですが，普段の自分は Scala プロジェクトで 300 行を超える変更が生じると頭を下げながらレビュー依頼を出し，700 行を超える PR はほとんど作っていない…はず…
Java だとボイラープレートが多いので，1,000 行超えるケースもあるかな，という感じです．

マージ先ブランチ上に rebase してから PR のレビュー依頼を出す (必須度: B)

大きい修正をするときにはどうしても競合は起きがちですが，先にあった「意味のないコミットログを作らない」という観点からも，更新されたマージ先のブランチに rebase したものをレビューに出すようにします．
先にも振れましたが，メンティーに仕事を振るときのメンターの気の遣いどころでもあります．

人によっても違うと思うし，書き忘れているところもあると思いますが，僕は上記のような感じでやっています．
皆さんはどうですか？