From ff2261b59ca2c936e423fb24a0a13eaaab047116 Mon Sep 17 00:00:00 2001 From: Junki Mano Date: Mon, 23 Dec 2024 15:27:31 +0900 Subject: [PATCH] =?UTF-8?q?=E3=80=90Git=E3=83=96=E3=83=A9=E3=83=B3?= =?UTF-8?q?=E3=83=81=E3=83=95=E3=83=AD=E3=83=BC=E8=A6=8F=E7=B4=84=E3=80=91?= =?UTF-8?q?=E3=83=AC=E3=83=93=E3=83=A5=E3=83=BC=E6=99=82=E3=81=AE=E3=83=95?= =?UTF-8?q?=E3=83=AD=E3=83=BC=E3=81=AB=E3=81=A4=E3=81=84=E3=81=A6=E8=BF=BD?= =?UTF-8?q?=E8=A8=98=20(#193)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../forGitBranch/git_branch_standards.md | 55 ++++++++++++++++++- 1 file changed, 53 insertions(+), 2 deletions(-) diff --git a/documents/forGitBranch/git_branch_standards.md b/documents/forGitBranch/git_branch_standards.md index fd143104..ba4e0a1d 100644 --- a/documents/forGitBranch/git_branch_standards.md +++ b/documents/forGitBranch/git_branch_standards.md @@ -261,6 +261,57 @@ featureブランチでの作業中に、developブランチが更新された場 ::: +### プルリクエスト作成前にアップストリームをプルする + +featureブランチの開発が終わり、プルリクエストを作成する際には、改めてアップストリーム（developブランチ）の変更をfeatureブランチに取り込み、差分が無いことを確認すべきである。 + +理由は次の通り。 + +- レビュアーの負荷軽減のため + - レビュアーがプルリクエストの差分以外の部分を参照した際に、それが古いバージョンであると、誤指摘、混乱してしまうなどの懸念がある +- マージ後のdevelopブランチでテスト失敗するリスクを減らすため + - コンフリクトせずにマージ可能だったとしても、何かしらの依存関係や整合性が狂い、マージ後のテストが失敗する可能性がある + +### プルリクエストのレビュー依頼までにどこまでテストしておくべきか + +本規約で推奨する `Lite GitLab Flow` `GitLab Flow` ともに、開発環境へはdevelopマージをトリガーにCI/CDでデプロイを推奨している。 + +そのため、プルリクエスト作成時点では開発環境（≒AWSなどクラウド環境の想定）へのデプロイ＋動作検証は不要である。 + +ローカルでの開発のみで品質担保が難しく手戻りが多い場合は、サンドボックス環境や開発環境にfeatureブランチからデプロイして動作検証する作業が必要になる。開発環境を共有する場合は、デプロイタイミングの制御がチーム内で必要になるため、運用ルールを検討する必要がある。 + +### Terraformはレビュー依頼時点でどこまで確認しておくべきか + +Terraformはplanが成功しても、applyが失敗することは多々あり（サブネットが足りなかった、force_destory=trueを明示的な設定が必要だったなど）、レビューでの見極めは難しいことが多い。そのため、applyをどのタイミングで実施するかがチームの生産性の鍵となる。 + +大別すると以下の3方式が存在する + +1. マージ後にapply + - PR -> CI（planを含む） -> レビュー -> developマージ -> apply(CI) +2. Approve後にapply + - PR -> CI（planを含む） -> レビュー -> apply -> developマージ -> apply(CI) +3. レビュー依頼前にapply + - apply -> PR -> CI(plan含む) -> レビュー -> developマージ -> apply(CI) + +それぞれの特徴を下表にまとめる。 + +| 観点 | ①マージ後にapply | ②Approve後にapply | ③レビュー依頼前にapply | +| -------------------- | ---------------------------------------------------------------------------- | --------------------------------------------------------------------------------- | ------------------------------------------------------------------------------- | +| 説明 | developブランチにマージ後にapply。アプリコードと同じメンタルモデルを共有可能 | レビュアー承認後にapply。featureブランチからapplyするため、あるべき姿からは外れる | レビュー依頼前にapplyで成功したことを確認する方式 | +| developブランチ品質 | ❌️一時的にapplyが失敗するコードが混入するリスク | ✅️apply可能なコードのみに保つことができる | ✅️apply可能なコードのみに保つことができる | +| レビュー負荷 | ❌️applyの成否は不明なので心理的負荷あり | ❌️applyの成否は不明なので心理的負荷あり | ✅️applyが成功している前提で対応可能。apply結果をコンソールからも確認可能 | +| apply失敗時のコスト | ❌️再度PRを作る必要があり手間 | ✅️同一PRを流用できる | ✅️apply成功後にPR作成が可能 | +| PRのトレーサビリティ | ❌️PRが割れると面倒 | ✅️同一PRである | ✅️同一PRである | +| 環境のバッティング | ✅️ない | ⚠️Approveからdevelopマージまでの間に、他メンバーの作業と重複するとややこしい | ❌️作業調整が必要 | +| ガバナンス | ✅️applyをCIのみに絞るなど自動化と相性が良い | ⚠️レビュアー承認後のコードのみapply対象とできる | ❌️ノーレビューのインフラ変更を適用するため、初学者が多いチームには適用が難しい | +| 結論 | applyの成功率が高く維持できる場合に有効 | applyの成功率が低い場合に有効 | 少数精鋭の場合に採用可能な、上級者向けの方式 | + +本規約の推奨は以下。 + +- 新規参画者が多く統制を取りたい場合や、applyの成功率が高く維持できる場合は①を選択 +- ある程度インフラメンバーが絞れ、かつapplyの失敗率が高くレビュー負荷が高くなってしまう懸念がある場合は②を選択 +- インフラメンバーが少数精鋭（通常、同時の作業はほぼ発生しない）の場合は必要に応じて、②をベースにしながら③を取り入れて生産性を上げる + ## 2. featureブランチからdevelopブランチへ変更を取り込む プルリクエスト（以下、PR）を経由して、開発が完了したfeatureブランチをメインのdevelopブランチに取り込むためには、GitHub（GitLab）上でPRを経由する運用を行うこと。 @@ -295,12 +346,12 @@ featureブランチでの作業中に、developブランチが更新された場 プリリクエストの承認（Approve）をもらった後、マージはレビュアー／レビュイーのどちらが行うべきか議論になる場合がある。 -| 観点 | レビュアー派 | レビュイー派閥 | +| 観点 | レビュアー派 | レビュイー派 | | ---------- | ------------------------------------------------------------------------- | ------------------------------------------------------------------------------------ | | 説明 | 開発者の責務が、developブランチにマージするまでという役割分担の場合に有効 | 各開発者がその機能のリリースについて責任を負うモデルの場合に有効 | | 生産性 | ⚠️レビュアーがブロッキングになりがち | ✅️高い。コメントはあるがApproveしたので、適時対応してマージして、といった運用が可能 | | 統制 | ✅️レビュアーが管理しやすい | ✅️メンバーの自主性に依存 | -| 要求スキル | ✅️低い。中央で統制を書けやすい | ⚠️開発メンバーの練度が求められる | +| 要求スキル | ✅️低い。中央で統制を行いやすい | ⚠️開発メンバーの練度が求められる | 上記にあるように、そのプルリクエストで実装した機能を、本番環境にデリバリーする責務をどちらに持たせるかという観点で、意思決定することが多い。