チーム開発時に決めた独自のルールを晒します。規模が大き目の開発をやる際に、有用かと思いますので、載せました。有用なgithubルールはこれからも洗練し、変更を加えて行く可能性があります。
前置き
このルールは絶対ではありません。運用上より良いルールが生まれたり、置き換わったり、必要なかったら消すなどはいつでも検討・実行されますのでコメントはお好きにどうぞ!それぞれのルールが目指す目的を達成することや阻害要因にならない場合、究極の事態においては、優先されるものではありません。
形骸化させないことが大事、.githubにテンプレートを定義しましょう。
ラベルについて
ProjectsのColumn
| Column | 詳細 |
|---|---|
| plan | やるかどうか未定なもの |
| todo | やることが確定したもの |
| doing | wip |
| review | レビュー待ち |
| done | 終了 |
Label
| Label | 詳細 |
|---|---|
| bug | 致命的ではないバグ |
| hotfix | 致命的なバグ |
| refactoring | リファクタリングしたい場所 |
| must | リリースに必要な機能 |
運用方針
ブランチ命名ルール
feature or ラベル名 (以下ラベル名)
feature: Planningされていない機能追加要件やされているがmustではない要件。
- masterマージ対象
ラベル名/milestoneに紐づくイシュー番号 (例: must/89) - それ以外
ラベル名/issue番号 (例: must/89)
ラベル名/issue番号_any (例: must/89_send_mail)
must・ラベルのプルリクルール
リリースまでに必要な機能は必ずマイルストーンの設定から始める。
- MileStoneのタイトルに期限・このMileStoneを達成することで実装される機能を設定する
- 作成予定のブランチ名でissueを建て、test要件をチェックボックスで箇条書する
- Issueで設定したブランチ名のブランチを作成する
- マイルストーン派生のissueのチェックボックスを満たすためにその時点でわかっている必要なIssueを立て、マイルストーンのラベルを設定する（複数人で取り掛かる場合、最初に細かくIssueを切り分ける）
- これ以降、マイルストーン派生のIssueと同名のブランチからチェックアウトし、プルリクを出す
- issueのテスト要件をチェックして、問題なければ、マージする。ブランチ名のイシューを閉じる(Close idで自動で閉じれます)。
- maserマージ前にdevelopに バグが発覚した場合は該当のプルリクをrevertする。その場合、Issueを立て派生ブランチに修正、または、修正プルリクを出してマージし、その後,developにマージし直す。
- masterマージ後に発覚したバグは、新たな機能追加として処理する。(新たにIssueを立てるかmilestoneを立てる)
これとは別に例外で、パッケージ、スキーマ、criticalなbugfixなどは単一コミットでdevelopにマージリクエストを投げます。以下は、イシュー立てる必要ありません。
- パッケージ、スキーマ変更の前にslackの#dev_must_readチャンネルに周知する。
- パッケージ、スキーマ変更の単体をコミットしdevelopにプルリクを出す。もし、ある程度の変更後に追加したい場合はdevelopからブランチをきり、package変更したものを、developに出す。その後、package変更ブランチを作業用ブランチに取り込む。ただし、developに混ぜても、他の開発メンバに支障が出ない範囲である場合に限り、作業ブランチにパッケージ変更をコミットし、developマージすることは認める。
- マージされたらslackの#dev_must_readでdevelopをpullするように周知する。
以下その状態を図示したもの。
なぜ上のようにやるのか
派生ブランチは治外法権,developのプルリクは、入国審査。明らかにでかいコードは派生ブランチでコードレビューを行い、入国審査を最小限にすることが目的。
またマイルストーンは計画全体の進捗を管理するためにつかう。masterはリリースタグがあれば良いかもしれないが、マージ前のdevelopブランチがあまりにもひどい状態になった際にブランチ切り直して、リリースできるブランチのみをマージするためのチェックポイントとしておいておく。
マイルストーン毎にイシューを切り分け、派生ブランチへのプルリクを挟むのは、開発中の機能をなるべく疎結合の単位で分断させ、リソースの足りない際に協力要請をしやすくできることと、レビューを細かく挟んで、一回あたりのレビュー量を減らしてレビューの質をあげるため。
それ以外のラベルのプルリクルール
developにプルリクしてAssigneeを設定して、コードレビュー受けたらマージ。
規模がでかければMileStoneを引くが、強制ではない。
コミットルール
プルリク済みのコミットに関しては以下の要件は不可逆的かつロールバックの単位はプルリクで吸収できるのであまり意識しすぎることもありませんが、綺麗なコミットを保つためにコミット時には、原則以下を意識してください。
- 1コミットに追加されている処理は全て密結合にならざるを得ないものの単位でコミットを行う(独立の機能として実装できるものは独立のコミットに分ける、余計なコミットを混ぜない)
- commit単位でcherry-pickしても、必要な変更点が抽出できる
- 一つのコミット内で追加した関数・メソッドについてテストを書く
- issue番号をコミットに紐づける。
ローカルコミットをpush前に治せるものに関してはrebaseを行い以上の粒度に合わせてからプルリクを作成してください。
全ての関数・メソッドに対してテストコードがある状態にしますが、testのpendingはカジュアルで大丈夫です。
得られる安全性に対してテストを書くコストの方が高くつく場合は、pendingにしておいてください。
プルリクルール
developへのプルリクは必ずアプリケーションが正常に動く単位でマージすること。これを守らないと問題を起こしてるマージコミットがわからなくなり個々のロールバックができないため、前バージョンまで戻すことになることになる可能性が高くなる。
また、主要機能はissueやマイルストーンで管理するため、同じマイルストーン上で作業しているブランチについては、プルリク自体はカジュアルに出して派生ブランチはアグレッシブマージしてもよいし、ガンガンするべき、未完成でもコードマージできるタイミングで、ガンガンマージする。
つまり、以下の条件を全て満たしていれば、主要の機能の一部の実装しか完了していなくてもプルリクを出しても良い。またコードレビュー者もマージしても良い。
devlop以外のマージルール
- 最低限機能が壊れていない
- pending状態でも担当した機能のテストコードがある
- milestoneのassigneeが好きなタイミングでマージしてよい。周りもそれに協力してガンガンマージリクエストを出す。
devlopマージルール
プリフィクス共通
- 追加された関数・メソッド全てに対してtestがされておりpassしている、あるいは、pendingだがpendingの理由が書かれている(だいたいどのテストフレームワークでもpendingの際に出力メッセージを設定できるので理由を出力してください)
- そのプルリクがある機能の一部分の実装である場合は先にissueやmilestoneに登録してある。
- プルリクブランチのHEADでコードが正常に動くことを確認した。
hotfix・bugプリフィクスの場合
- バグの原因となったコードに対して、同じバグを起こすコードを書いた場合、テストが失敗するようにテストを追加する。
以上を満たしていない場合は、原則マージ不可とします。
また、以下の場合は、絶対マージしてはいけないというわけではないですが、change requestを検討してください。
- 10commitを超えそうかつ変更点が多い場合(マージできるコミットまでプルリクを分けることを検討する。)
- 一つの関数内に複数の関数に分けるべき処理が入っている
- 極端に長いメソッド
- もっと良い設計に変えられる場合
- privateでいいデータがpublicになっている
- ファイル階層が情報の粒度とそろっていない
- 必要以上の変数定義
- 関数の引数を無駄に多く使用している
- 危険なオーバーライドやオーバーロード関数の使用
- thisの中身が不明瞭
- staticにすべきinstanceメンバや、その逆
- メソッド名や変数名が誤解を招きそうな表現であったりわかりづらい
- pending中のテストコードの実装コストが低そうな場合
- typo
- コーディング規約を無視したコードがある
例外
やる必要のないtestはカジュアルにペンディングしておいて良い。
ただし、チームメンバー全員でpendingになっているテストを
余裕があればpassさせていく。
最初のプロジェクト作成時のコミットの大きさはある程度許容する。
緊急時
リリース直後に発覚したばあい、前回のリリースまでコードを反映させる