はじめに
私は2023年3月でエンジニア2年目を終え、3年目を迎えます。
普段プルリクエストベースで開発してて「これ辛いな」と思うことがあり、それに対してやってみたことを書きます。すでにやってる方が多そうな初歩的内容ですので暖かい目で読んでください。
「自分はこんなふうにしてるよ!」というご意見あれば是非コメントください。
目次
- プルリクエストで気になること
- 気になることを解消するために意識したこと
- 意識したことでよかったこと
1. プルリクエストで気になること
1-1. プルリクエストのFiles changedがデカい...
自戒も込めてます。自分は20ファイル超えるとキツイです。
軽微な修正であってもプルリクエストのリンク開くと「おぉ...」となります。
上記2つとも自分が過去に作ったプルリクエストです。「そんなに多くないじゃん?」と思う方もいますが、これが連続でやってきてかつ短い期限でレビューしなくてはいけないという状況だと結構難しいものです。40ファイルの差分が来た日には結構気合入れてレビューに臨むことになります 笑
どうしても分けることのできない場合もあります。ただ、分けられる時は分けたいものです。
巨大なプルリクエストはレビューする側に負荷をかけミスや見落としにつながると私は思ってます。
1-2. commitメッセージがわかりにくい...
下記はサンプルで筆者の悪ふざけが過ぎてますが、一見よくわからないcommitメッセージが作られたりします。
- ひとまずcommit => 「ひとまず」ってなに? 何をしたの?
- 2023/03/01分 => 日記? ただの自分日記を作られても困ってしまう
- 頼む => なんだろう... このコードを書いてる時の心情が見える....
プルリクエストをmergeする際、commitをまとめずにmergeしてるならば後ほどcommitを見返す時に一つずつ見ていく必要が出てきます。せめて「何をしたのか」はわかるようにしたい。
2. 気になることを解消するために意識したこと
2-1. 「プルリクエストのFiles changedがデカい...」に対して
プルリクエストを分ける
1個のプルリクエストで全部やろうとしない!!
段階に分けられそうなら分けましょう。例を挙げます
1. スキーマの変更(Swaggerの修正)やDBの変更(マイグレーションファイルとかSQLとか)
------ レビュー ------
2. プロダクションコードの実装
------ レビュー ------
3. テストコードの実装
------ レビュー ------
レビューをこまめに挟み、早めの確認を行えばレビューアーさんの負荷が減らせます。
ボーイスカウト・ルールの対象範囲を決める ※ボーイスカウト・ルールについて
例えばUserController
に機能を追加/修正するタスクがあるとしましょう。UserControllerをいじるタスクなのでUserControllerから出ないようにしましょう。テストコードもそうです
例)ここにあるファイル以外は無理に触らないようにする(少し極端な例を挙げています)
apps
├── Controllers
| └── UserControleler
|
└── tests
└── UserControleler
├── IndexTest
├── StoreTest
├── ShowTest
├── UpdateTest
└── DeleteTest
関連が深い場所に手を加えた場合、レビュー側に追加の負荷はかかりにくいです。関連性の浅いものを一緒にレビューしてもらう場合、頭の切り替えが多くなるので負荷が高まります。
ボーイスカウト・ルールは大事です。しかし、「ここもここも...」と手を出してしまうと無限にできてしまいます。 無限にできるということはプルリクエストに含まれるものも無限に増えてしまいます。せめて範囲は絞りましょう
2-2. 「commitメッセージがわかりにくい...」に対して
commitメッセージにprefixをつける
自分がつけるprefixと意味は下記の通りです
- add => 新規ファイルの追加
- fix => バグ修正
- update => ファイルの更新
- delete => ファイルの削除
- ex => merge, submoduleの更新などgit処理
例)
git commit -m add: Userモデルの実装
git commit -m update: swagger.yaml get:/api/usersのレスポンスに〇〇を追加
git commit -m ex: mainブランチの取り込み(merge)
とかですね。他に適切なprefixがあれば追加しても良いです。
3. 意識したことでよかったこと
3-1. 相手のレビュー負担が減る
どちらのレビュー依頼が良いでしょうか
- 30ファイルのレビューが一括でやってくる
- 5ファイル, 15ファイル, 10ファイルのレビューが小出しでやってくる
自分は2番が良いです。一回のプルリクエストが平均10ファイルですし、気持ち的に余裕が生まれ良いレビューが出来そうな気がします。レビュアーしてくださる方はお忙しいことが多いです。少しでも負荷を減らせるように気を配りましょう。
※いきなりプルリクエストを小出しにするとレビュアーが驚くかもしれないので事前にコミュニケーションをとっておきましょう(画像に某キャラクターが見えてたので隠してます 笑)
3-2. レビューがすぐ終わる
レビューを小さく分けてるのでレビューにかかる時間が減ります。そうするとApproveは早くなります。
Approveいただけると嬉しいですよね。Approveのためにコードを書いてる訳ではないですが、「自分はここまでできている」という実感が持てるのでモチベーションが上がると個人的に考えています
3-3. レビュアーのレビュー方法が広がる
プルリクエストをFiles changedで見る方もいればcommitレベルで見る方もいます。チーム内でレビューする時のルールを決めておけば問題ないかもしれませんが、なかなかそこまでルール化できない場合もあります。そうするとレビューの仕方は「レビュー者次第」になります。commit単位でもレビュアーがレビューできるようになります。
4. まとめ
「プルリクエストを作って誰かにレビュー依頼をする」、「自分がレビュー依頼を受ける」を経験し、感じた疑問とその対応をまとめてみました。自分にとってレビューは結構パワーを使うことです。これからもレビュー負荷を下げられるような工夫を重ねてより楽しく開発できるように努力していきます。