プルリクエストを出す際のポイント
実務では、プルリクエストを出す際に注意すべきお作法があるのでこちらにまとめて起きます。
この作法は、マナー的な実利に関係ないものではなく 「チームや個人の生産性」に関わるもの のなので、 普段から気をつけて実践できると良いです。
プルリクエストを出す際に下記ポイントをチェックしてプルリクエストを作成してください。
注意点
- プルリクエストの目的を明確にする
- タスクを始める前に出すプルリクエストの本数やサイズを考える
- 一プルリクエストのサイズを抑える(追加・削除合わせて1000行以下)
- セルフレビューを行う
- レビュアーがわかるように変更内容を説明する
- 一つの指摘に対して他に修正が必要な箇所がないか見直す
- レビュー指摘を理解して修正を行うまたは修正方針を相談する
1. プルリクエストの目的を明確にする
プルリクエストを出す際には、そのプルリクエストの目的を明確にすることが重要です。
一プルリクエストに一つの目的を持たせるようにしましょう。
例えば、課題を進める際に全ての実装を一つのプルリクエストにまとめるのではなく
- 機能ずつ
- 指摘の修正単位
でプルリクエストを作成するようにしましょう。
目的の不明瞭なプルリクエストは、サイズが肥大化しやすく、多くの変更要因を含んでいていレビューが難しくなります。
2. タスクを始める前に出すプルリクエストの本数やサイズを考える
実装が終わった段階でプルリクエストについて考えるのではなく、タスクを開始した時にどのようなプルリクエストにするか考えましょう。
変更のサイズが大きくなりそうな時は、プルリクエストを段階に分けて作成しましょう。 1からプロジェクトを始める場合などは、
- プロジェクトの雛形の作成
- Eslint+Prettierの設定
- 機能Aの実装
- 機能Aの実装
... n. 機能Xの実装
のように段階を分けてプルリクエストを作成することで、レビューを効率的に進めることができます。
3. 一プルリクエストのサイズを抑える
一プルリクエストのサイズを 追加行、削除行で 1000行以下に抑えましょう。 追加行 500行、削除行 500行程度で合計1000行になります。
大き過ぎるプルリクエストは、複数の変更要因が含まれている可能性が高いので、全体を見渡すのが難しく、指摘があった場合に雪だるま式に複雑度が上がってしまいます。 プルリクエストのサイズを抑えることで、自動的に変更の複雑度が上がりすぎないようにできます。
タスクを始める時や、実装中に変更のサイズを都度気にしながらプルリクエストを区切って作成するように注意しましょう。
レビューを繰り返してプルリクエストのコメントやサイズがふえてしまった場合
二度、三度と同じ変更のレビューを繰り返すとどのコメントがどの修正に対応しているのかわからなくなります。
学習用のレポジトリやリリース前のオリジナルアプリのようなmain, develop ブランチにマージしても問題がない場合は、 一旦マージを行い各修正用のプルリクエストを作成してレビューを受けるようにしましょう。
4. セルフレビューを行う
レビューを依頼する前に必ずセルフレビューを行いましょう。
- console.logやdebugger などデバッグ用のコードが残っていないか？
- 不要なコメントが残っていないか？
- 不要なファイルがコミットされていないか？
- 以前指摘された内容と同内容の誤りを犯してないか？
- スタイルは統一されているか？(大文字、小文字、インデントなど)
- コミットされていないファイルがないか？
- ロジックの重複がないか？
- 実際の処理内容と関数名・変数名が一致しているか？
のように未経験、経験があるに関わらずすぐに意識できるものはレビューに通す前にチェックして自分で修正できるようにしましょう。 自分がコードレビューをする側に回った時の練習にもなるので、必ずセルフレビューを行うようにしましょう。
5. レビュアーがわかるように変更内容を説明する
GitHub ではプルリクエストの冒頭に説明をかける場所があるので、
- 変更の要因
- 変更の目的
- 変更の内容
- 変更の影響範囲
- レビューの観点（どこをチェックして欲しいか）
などを記載してレビュアーが
- どのような変更があったのか
- なぜ変更が必要だったのか
をすぐにわかるようにしましょう。
GitHub では、差分のページで行単位でコメントを残すこともで切るので、部分的に説明が必要な場合は行単位でコメントを残すようにしましょう。
6. 一つの指摘に対して他に修正が必要な箇所がないか見直す
レビューで指摘を受けた際に、指摘を受けた箇所以外にも同じ理由で変更が必要な箇所がないか確認しましょう。 言われた部分だけ直すのではなく、常に「他に修正が必要な箇所がないか？」確認して修正を行いましょう。
横展開した修正の量が多い場合は専用のプルリクエストを作成して、元々のプルリクエストのサイズを肥大化させないようにしてください。
7. レビュー指摘を理解して修正を行うまたは修正方針を相談する
レビュー指摘を受けた際には、レビュワーからの指摘を一方的に受け入れるのではなく、指摘内容を理解して修正を行うか、修正方針を相談するようにしましょう。
簡単な内容であれば、すぐに修正を行っても良いですが、 設計レベルの指摘では修正方針を考えて 「このように修正しようと思っていますがどうですか？」 と確認するようにしましょう。
修正方針が想定されていたものと違う場合は、二度も三度も修正が必要になって必要以上に時間がかかってしまうので修正方針の合意をとってから修正しましょう。
レビューはインタラクティブに行うことが大切なので、指摘だけを黙って読んで修正するみたいなことはなるべく避けましょう。
NG集
以上のポイントを踏まえて、プルリクエスト周りのNG集を挙げておきます。
- 一つのプルリクエストに複数の目的がある
- 一つのプルリクエストに複数の機能が含まれている
- 指摘の修正を一つのプルリクエストで済ませようとして、どの修正がどの指摘に対応しているのかわからなくなる
- console.log の残りや不要なファイルなど指摘する必要ない項目が残っている
- 同じ内容の指摘を複数回受ける
- レビュアーがわかるように変更内容を説明していない。説明がない。
- どこをチェックして欲しいかを明記していない
- 指摘された箇所だけ修正している
- レビュー指摘を理解せずに修正して、また同じ指摘を受ける
まとめ: 実務を想定したプルリクエストを作成する
プルリクエストを出す際には、実務を想定したプルリクエストを作成することが重要です。
上にあげたような注意点は実際の現場でも気をつけるべきポイントなので働き始める前に習慣づけられると良いです。
実際に仕事をするときに、自分の成果物をレビュー依頼する際にセルフチェックをせずにレビュー依頼するようなことはしない と思いますが、いざコードを書いていると達成感からセルフチェックを忘れてレビューを出してしまうなんてこともあります。 説明もなしに「確認お願いします」とだけ人にレビューを依頼することも実際の仕事ではないことだと思います。
プログラミングでもこのような仕事の上で気をつけるべき部分は同じなので、常に「これが仕事だったらどうするか？」と考えながらプルリクエストを作成するように すると自然と確認のしやすいプルリクエストを作成できるようになると思います。。