PRのつくり方を考える

ここ数年くらいPRのつくり方について、尊敬する先輩に教えて頂いたりと色々試行錯誤があったので折角なので纏めていく。

TL;DR

  • big commitは避ける。1story /taskに対してPRは複数用意していい。とにかくbig commitは避ける。
  • review結果は反映するものではなく取り込むもの。
  • Testは頑張って書く。ひたすら頑張って書く。良いコードを一発で書けないからこそ頑張って書く。
  • コード解析やフォーマッティング等、気付きを自動化できる仕組みは積極的に使う。楽をして、浮いた時間は別なことに使う。

Pull Requestの処理にかかるカロリーを考える

(reviewee)Pull Requestを出す
(reviewer)観測する
(reviewer)内容を把握する
(reviewer)指摘を書く
(reviewee)Responseを書く or 修正のコミットを積む

のラリーを行うため、非常にカロリーが必要な業であることをまず理解する。 また、責任分散の目的を考えると2approve以上が望ましいため、reviewerのカロリーを下げることが非常に大事。

big commitをなるべく避ける

big commitというかbig pull requestというか。。。
change setをなるべく小さく、小口でPRを作成することが大事。

なぜbig commitを避けるのか

big commitは

  • review業がそもそも大変になる
  • 大変になるとどうしても見落としが出てくる
  • 大変なので後に回されやすくなる

といった感じになると実感している。

1Story / Task に対して 1PRである必要は全然ない

1PRは1Storyじゃなくて良い
という気付きを得てから、改めて理解したが、人間が気を付けられる量なんてたかが知れている。

例えば50ファイル、3000Lineとかchange setを出されたところで、正直見ようと思えなかったりする。(仕事だから見る努力はするけど、実際大味なレビューになるんですよね。)

なので、

  • コードベースを良くしていきたいのであれば、PRはどんどん分けたほうがいい。
  • 一方で、PRをチェインすると、最初のほうのPRで指摘があるとrebase業が死ぬほどダルくなることがある。
  • この辺は、git力を少し上げることで改善が可能なので、少しずつ頑張る。

みたいな感じで進めていくのが筋が良いように思う。
特に、git力については避けて通れない。もしチーム内でgit力が足りていないと感じたらラーニングしていくと運用が改善していくのでオススメ。

review結果は反映するものではなく取り込むもの

レビュー結果反映とか指摘結果反映とかレビュー指摘事項修正みたいなcommit comment
は、

  • 対応内容が明示されていないことが問題
  • レビュー結果について納得せずに、「はいはい、指摘されたから直しましたよ、これでいいんでしょ。」っていう感じになるのが問題

の2つの意味で筋が悪い。 特に後者の気持ちでは、人は育たないので気を付けたほうが良い。指摘事項反映(詳細な文章)みたいなのも、後者の問題に引っ掛かるので筋が悪い。
レビューの指摘結果は意図も含めて取り込むものであるという前提を忘れないようにしたい。

Testを頑張って書く

品質を語るならまずTestを書け。は本当にその通りで、

  • PRを出すときにはゴールデンパスのTestを頑張って書こう。
  • 業務アプリケーションの場合は永続化層込のTestを頑張って書こう。

が本当に大事。 reviewでもtestがなければ超緊急時を除いてapproveしないほうが筋が良い。

UNDONE: Testの効率とかは、まだまだ考えられてない

今の時代、Docker上での自動Testが主流なのだから、今のところ雰囲気で使ってるDockerをもっと理解することで改善が進むんだと思う。

dependabotはいいぞ

dependabotは、自動Testがいてくれることで、nuget packageのアップデート時のリスクがある程度何もしなくても可視化されてく感じがして素晴らしいと感じている。
一方で、どうしてもreleaseまでのgit のbranch modelを考えると、mainへのPR作成は、良し悪しというか、チームのパワーには若干合ってないとも感じるので、実際は、自分でPR作りなおすこともままある。(これはたぶん私が雰囲気で運用しているからというのもあると思う。)

github actionsでも、dotnet formatをかけて差分をPRにするworkflowを動かすようにしているが、結局のところコードベースがヘルシーな状態かどうかのインジケーターなんて多くて困ることはないからどんどん頑張ればいいんだと思う。