こんにちは！株式会社Pentagon代表の山本です。弊社のプルリクエストの運用に関して説明する機会があったので、今回「プルリクエストの意味とメリット・注意事項」についてまとめてみました。
本記事を読み終えると、プルリクエストを作成するときに気をつけるべきことがわかります。
プルリクエストによるコードレビューをする意味・メリットとは？
そもそもプルリクエストを作成する理由ってなんでしょうか？
ソフトウェアの品質を担保・バグを未然に防ぐため
プルリクエストをつくり、他のエンジニアにレビューしてもらうことで、ソフトウェアの品質を担保することができます。１人で作業していた時には、気が付かなかったバグが他のエンジニアにレビューしてもらうことで、発見されることもあります。レビューの時点でバグを未然に防ぐことができます。また、コードやロジックの書き方などを見直す機会にもなり、保守しやすいコード・バグが発生しにくいコードを書くことができます。
プルリクエストにより、「実装 → 確認 → テスト → バグ修正」という一連の流れをプルリクエストという小さな単位で行うことができます。これにより、プロジェクト全体では、何度もテストが繰り返されるので、バグや未実装の箇所を洗い出すことができ、ソフトウェアの品質向上に繋がります。
未実装の部分を洗い出すため
どれだけ優れたプロジェクトマネージャーだとしても、はじめからすべてのタスクをチケットとして書き出し、管理することは難しいでしょう。締め切り間際になって「この機能が実装されていなかった」なんてことも有りえます。プルリクエストをレビューすることで、「まだ実装していない＆チケットとして書き出していない項目」をあぶり出すことが可能です。もちろんこれには、エンジニアの積極的にレビューする態勢が必要となってきます。
スキルアップするため
上級エンジニアからアドバイスをもらうことによって、「こんな書き方もあるのか」と勉強になります。プルリクエストを積み重ねることで、スキルアップが期待できます。これによって、開発チーム全体のスキルアップを図ることができます。
プルリクエストテンプレートを設定する
プルリクエストを書きやすいように、pull_request_template.mdを追加しましょう。プロジェクトルートから見て、.github/pull_request_template.md を追加します。
## 対応issue
resolve #
## 変更内容
## 別チケットで対応する項目
## その他(UI変更あれば画像添付)
pull_request_template.mdを追加することで、githubにてプルリクエストを作成する場合、テンプレートとして扱われます。これによりプルリクエストの記述がしやすくなります。また、「resolve #100」といったようにIssueナンバーを書くことで、マージするときに自動的にIssueをCloseしてくれます。
レビューしてもらう前にチェックしておきたいこと
使ってないコードが含まれていないかどうか
開発中に使っていたけど、使わなくなったコードがあれば削除しましょう。使っていない無駄なコードは、エンジニアのプログラムの理解と行動を妨げます。
ただし、開発に必要だったログ出力は残しておいても良いでしょう。デバッグするのに役立ちます。
命名がわかりにくくないかどうか
プルリクエストを作成するときに、冷静になって、もう一度、変数やクラス名がわかりにくくないか確認しましょう。
変数やメソッドの名前は、誰が読んでもわかりやすいような命名にすることをに気をつけましょう。言語にもよりますが、Swiftであれば、Booleanの変数名には、「is」「should」などをつけて、疑問文になるようなコードにすることをおすすめします。良いソースコードを書くための11のポイント【スキルアップしたいエンジニア向け】
読みにくくないかどうか
もう一度、自分が書いたコードが読みにくくないかチェックしましょう。他のエンジニアが見たときに理解しやすいコードになっているかどうかがポイントです。If文が多段階でネストしていたり、１つのメソッドが長かったり、「パッ」と見た時に見やすいコードでなければリファクタリングの余地があるかもしれません。
デザインと一致しているかどうか
弊社では、Figma・Zeplin・InVisionといったツールでデザインを共有していますが、自分が実装した画面やUIパーツがデザインと一致しているか確認しましょう。異なる画面サイズのデバイスで動作確認して問題ないかどうか
iPhone Xの画面サイズで開発していて動作に問題がないものの、iPhone SEの画面サイズでは正常にUIが表示されないなど、アプリ開発では、画面サイズの違いによる不具合が起きやすいです。必ず複数の画面デバイスで動作確認をしましょう。 例えば、スクロールが必要な画面では、スクロールできるようになっているか確認することも重要です。 また、アプリ開発では、実際に文字入力をしてみることも重要です。キーボードでUIパーツが隠れてしまわないか、入力しやすいか、など操作感もチェックするポイントです。TODO/FIXMEコメントは適切かどうか
TODOやFIXMEといったコメントを書くことで、まだ実装が完了していないことを示したり、今後改善が必要であることを示すことがあります。 このとき、他のエンジニアが見ても、TODOやFIXMEの内容が明確であるかどうか確認しましょう。変更内容は明確か
プルリクエストの説明をちゃんと書きましょう。対応しているIssueはどれか、どんな変更をしたのか、UIの変更があればスクショを貼っておくとレビュワーも確認しやすいでしょう。 また、１つのプルリクエストの目的は、できるだけ１つにしぼることで、変更の意図を明確にします。新規でアプリを作成する場合は、ある程度まとまった変更をすることも良いでしょう。別issueで対応することがあれば明記する
Issueの対応をしている時に、「ここも修正した方が良い」と思うことが多々あると思います。そんな時は、一度、今のプルリクエストに変更を含むべきかどうか考えてみてください。 もし別Issueとして対応した方が良い場合は、必要に応じて Issue を作成するようにしましょう。TODOやFIXMEのコメントだけでは、対応するのを忘れたままプロジェクトが進行してしまう恐れがあります。作業途中でもDraftプルリクエストを出しましょう
まとめ
プルリクエストは、以下の点で重要なワークフローになります。- ソフトウェアの品質を担保・バグを未然に防ぐため
- 未実装の部分を洗い出すため
- エンジニアがスキルアップするため
- 進捗状況を把握するため
