PR運用について - lanchester/rails_environment GitHub Wiki

PRの必要性

なぜPRチエックをやるのか

目的

• 質の良いコードを保つ
• ミスを回避する
• 情報共有

どのような効果があるか

• リスクがあるコードの回避
• 処理が重い、データ件数が増えると極端にパフォーマンスが悪くなるなど)
• 単純なミスの回避
• (スペルミスや仕様の勘違いなど)
• 重複したコードの回避
• (機能的にはすでに存在するメソッドの定義など)
• 質の悪いコードの回避
• (無駄に冗長なコードや、定義場所がおかしいメソッドなど)
• コーディング力の向上
• 他人のコードを見ることにより、コーディング能力が向上する
• 状況の把握
• 自身の担当外のところをおおまかに把握できる

ルール

• 作業途中のコードは先頭に[WIP](work in progress)と付ける事とし、WIPのPRはマージしてはいけない
• 原則、実装者全員が見る事とする
• 「見た」の定義は「みました」とコメント、もしくはLGTM画像を貼りつける事とする
• 手直しが発生するなどマージされてはいけない状況になったらすみやかにWIPを付ける
• 全員が「見た」段階で最後に見た人がマージする
• 確認しやすいよう、PRあたりのファイル変更は10ファイル以下になるように努力する
• PRを1日以上放置しない
• コミットログは目的、理由、意図がわかる内容にする
  • 例えば「Lectureモデルの医師の一覧表示のソート順を変更」など
• コミットの粒度はキリの良い所で小さくする
  • わかりやすい基準としては、なにがしかの理由でチェリーピックするようにった場合困らないような状態
• 見てもらいたいポイント、見てもらいたい人、重要度などはPRに書くようにする
  • コミットログから読み取れないところはPRに書くようにする
• マージする人は予め定義しておく（複数人いると作業が止まらなくて良い）
• PRに対するコメントはソースコード上の特定の行につけられないようなものはチャットツールなど別の場所で議論する
• 一度PRを見終わった人に対しても再度見てもらいたい場合は見てもらう側が再度確認依頼を出す

運用方法

• 作業が詰まっていて見られない場合 ⇛ 見られない旨をコメントに書いて「みました」する
• スキル的に見るのが不可能 ⇛ 見られる部分(例えば文言のミスなど)を見て「みました」する
• 急いでマージしたい場合 ⇛ チームに対して周知(相談)して問題なければPR出した人が自己マージする

ポイント

• PRを見てロジックが難しくてわからないところがあっても気軽にコメントで聞くようにする
• 全てのPRをしっかり見ようとするとPRチェックに時間とられすぎて破綻するのでわからない時や時間がとれないときは事情を伝えた上で、迷わず「みました」してしまうという「わりきり」が大切です
• 変更ファイル数が多いと極端にチェックがしづらくなるので、Issueを細かく分けてPRの単位を小さくするほうが良いです
• Hound CIの対応はほどほどにする
• Githubの画面上で編集するとPushできなくなったり困ることが多いので原則GitHub上では変更しないようにする