PR Review Process

[cipherboy]

Loosely modeled off of Kubernetes:

---

The Main Code Review Process

• Phase 0: The author submits a PR to an OpenBao repository's main branch.
  • PRs to feature branches and release branches are not covered by this process.
  • GitHub tags code owners based on the Author's modifications. See the CODEOWNERS file in the target repository for more information. For instance, the CODEOWNERS file for openbao/openbao assigns PRs which change builtin/logical/kv/ to @openbao/openbao-org-maintainers or @openbao/openbao-core-committers.
    • Only one code owner for a given file needs to approve the PR, but PRs may modify files with disjoint code owners and thus require multiple approvals.
  • Anyone with at least triage permissions on the repository may assign labels and milestones to the PR. Milestones are always required for any code (non-documentation, non-CI) change.
    • Milestone should be set to the release that main targets. For PRs which need a backport, assign the needs-backport and backport-<version> label.
• Phase 1: All required human reviewers review the PR.
  • If the author is a required reviewer, self-approval does not count towards criteria.
  • All PRs must be reviewed by a reviewer at a different entity than the author. E.g., if an ExampleCorp author submits a PR review, a ExampleCorp reviewer's review would not count towards the required approvals.
  • If changes are desired and the reviewer commits to seeing through the revisions, use of changes required is appropriate. If the reviewer does not have capacity, using the comment functionality is preferred as stale changes required reviews cannot be dismissed.
• Phase 2: All required human approvers approve the PR.
• Phase 3: Merge the PR!
  • Any author or reviewers or approvers with permission may squash merge the PR into main.
  • Only in the event of feature branches with clean commits from multiple authors (which are without merge commits) will a branch be rebase-merged.
  • PRs which have the label blocked cannot be merged, even with approvals.
  • After merge, the author should be thanked. If they are a first-time author, please note this for the next community call.

The Backport Code Review Process

The backport process differs from the main process in the following ways:

• Without good cause, PRs should target main first. PRs not targetting main but targetting a release branch directly (with cause) should follow the above procedure for review+merge.
• PR approvals for backports need not be from a different organization.
• Milestone targets the next milestone of the given release branch.

The Feature Branch Code Review Process

Merging to a feature branch differs from the main process in the following ways:

• Code owners do not apply.
• Reviewers should be from a different organization, but need not be.
• The full review process must be followed for merging the feature branch to main.

[cipherboy]

From 10/30: need to confirm whether rebase merging (losing signing) is OK with various requirements or if we should use merge commit (which has signature on top, but may not have intermediate signatures).