From 8eb20b245ebcb7d4842f9ec03a8c91831477f307 Mon Sep 17 00:00:00 2001 From: Karl Baker Date: Fri, 29 Mar 2019 12:47:00 +0000 Subject: [PATCH] Update guidance on merging PRs to include Dependabot This commit updates the guidance on merging PRs to include Dependabot specific advice, in particular to recognise Dependabot as an external contributor and to ensure two approvals when necessary. This commit also updates the guidance on deploying changes after they have been merged to emphasise their deployment to production as soon as possible, practicing continuous deployment and ensuring that we can release our apps at any time without worry of a backlog of unreleased changes. --- source/manual/manage-ruby-dependencies.html.md | 2 ++ source/manual/merge-pr.html.md | 12 +++++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/source/manual/manage-ruby-dependencies.html.md b/source/manual/manage-ruby-dependencies.html.md index f6f60074fd..a474b1fe46 100644 --- a/source/manual/manage-ruby-dependencies.html.md +++ b/source/manual/manage-ruby-dependencies.html.md @@ -21,6 +21,8 @@ To help with this, we use a service called [Dependabot][] to perform automated d You can ignore pull requests from the bot by replying `@dependabot ignore this major version`, but you have to add the PR to the [tech debt Trello board][tech-debt] +If a PR contains a mixture of GOV.UK-owned gems and other gems (which are not solely included in the `test` block of the Gemfile), it will need 2 reviews. + ## Add Dependabot to a repo 1. Give Dependabot [access to the repo][access] (only GitHub org owners can do this) diff --git a/source/manual/merge-pr.html.md b/source/manual/merge-pr.html.md index c6e29fb598..37ae4f72c1 100644 --- a/source/manual/merge-pr.html.md +++ b/source/manual/merge-pr.html.md @@ -4,7 +4,7 @@ title: Merge a Pull Request section: GitHub layout: manual_layout parent: "/manual.html" -last_reviewed_on: 2019-02-15 +last_reviewed_on: 2019-08-28 review_in: 6 months --- @@ -16,6 +16,8 @@ There are five rules for reviewing and merging PRs, which apply to all applicati 4. The GitHub review UI should be used to mark a PR as approved or requiring changes. 5. The GitHub UI should be used to merge the PR. This ensures the PR number is added to the merge commit. +Once a PR is merged, you should deploy your changes at the earliest convenience to ensure unreleased changes do not back up and to keep our applications deployable - regardless of the perceived size of the PR merged (including [Dependabot](#Dependabot) PRs). [Deploying](/manual/deploying.html) should be done in the regular way, taking the merged changes all the way through to production. + ## Example scenarios ### A simple change @@ -64,6 +66,14 @@ git push --set-upstream origin thomasjhughes-patch-1 Change `thomasjhughes` to the GitHub username of the contributor and `patch-1` to the name of the branch in their fork. +#### Dependabot + +Dependabot raises PRs whenever it sees new versions of gems available that are required by our applications. + +Dependabot is an external contributor and is therefore subject to the same due diligence checks set out above as any other external contributor would have to go through, requiring two people from GDS to approve the PR before it can be merged. Particular attention should be paid to the changelog(s) of the upgraded gem(s) to ensure that no unintended side-effects are introduced. + +The exception to the number of GDS people required to review the PR is if the gems being upgraded consist of **only** GOV.UK-owned gems or gems in the `test` block of the Gemfile. If the PR includes _any_ external gems which are not in the `test` block of the Gemfile, two approvals must be obtained. See [Manage Ruby dependencies with Dependabot](/manual/manage-ruby-dependencies.html) for more information. + ### A change where two people worked on the same branch If two developers worked on the same branch and individually contributed commits, they can approve each other's work. If they worked as a pair, that pair counts as a single contributor, so someone else should review the work.