Pull Requests
When engineers have reached a point where they believe they are code complete, it is time to integrate changes from the source branch into target branch by submitting a pull request (PR). Apart from local builds and tests, PRs will provide an additional layer of protection for build validation as well as provide a collaborative platform for peer reviews.
Responsibilities¶
Pull requests are not a native Git feature and are performed on the Git server, which provide visibility for collaboration and validation. Once the developer performs a local build, they can push their changes and submit a pull request.
Submitter | Reviewer | Tool | |
---|---|---|---|
Information | Provide a meaningful title description | Review linked ticket to ensure acceptance criteria was met | Bitbucket links code change to Jira ticket |
Code Review | - | Review commits and file changes for standards | Pipeline run lints, scans, and tests |
Builds | Run builds locally before pushing | - | Bitbucket displays build status from CircleCI workflow |
Gating | - | Add comments; mark as Needs Review or Approve | Bitbucket provides visibility and enforcement for tasks |
Peer Reviews: A Sacred Space for Developers¶
Apart from quality gates, PRs provide an opportunitiy to perform honest, robust, and friendly code reviews. It is a sacred space often utilized only by the developers, as it narrows the audience to low-level code reviews. During PRs, developers will work together to ensure that the code meets the business value promised by the work item. Common discussions will revolve around scope creep, optimizing implementation, and alignment on team's code norms.
Pull request history persists on the Git server. Branches will be deleted and commits may be squashed, but the PR will persist to provide transparency on changes. All details will be retained, including code changes, reviewers and approvals, status changes, and comments.
Additionally, this process will help share knowledge of the application across the entire team, as opposed to a select circle of lead developers. This helps reduce tribal knowledge and can serve as an onboarding newer or junior members to the team. Good peer reviews during pull requests help create strong and high performing teams that:
- check each other's work to protect the quality of the product
- hold each other accountable
- over-communicate and over-share information as often as possible
- stay aligned on both the business value delivered and its implementation
- build a higher level of quality into the product early in the development process
Recommended Repository Settings¶
- Branch permissions: teams should have a strict agreement to protect the
main
branch both in practice and in policy. For teams following trunk-based development practices with short-lived branches, the following branch permissions formain
are recommended: - Required reviewers: this should be at least one but no more than two in order to provide faster flow. Specific individuals should not be required unless the specific PR requires review from a SME.
- Minimum approvals: teams should establish a minimum of one reviewer - but no more than two - to move fast. Teams may opt to require two approvals with one approval from the default list of reviewers (see below). This would be appropriate when teams want to add an external SME as a reviwer for specific PRs.
- Successful build: teams should enable branch CI builds and require passing buidls to validate changes before merging to
main
. - Reset status after source branch has been modified: if a PR has been approved and then changes are added to it, the status should reset to prevent merging unreviewed changes.
- Automatic merges: in an effort to move quickly, some teams may enable automatic merges, while some teams opt for more manual checks before merging. For teams first establishing their modern way of working, we recommend leaving this option unselected, but re-visiting in the future as teams mature.
- Default reviewers: the entire dev team should be added as default reviewers.
- Merge strategy: we recommend squashing upon merging to provide a clean Git history with meaningful, deployable commits.