Skip to content

Commit Management

Mathieu Hofman edited this page Oct 7, 2024 · 12 revisions

Because CI takes so long, we use a bot to facilitate landing changes in branches under branch protection rules such as master and hopefully in the future, release related branches.

The bot and merge process is currently coordinated by Mergify, and triggered by automerge:* labels. While GitHub will still allow you to manually land a PR if all conditions are met, we ask that you do not use the GitHub "Merge" UI button or GitHub CLI to land a PR, and solely rely on the Mergify system.

Pull request "merge" instructions

Once the PR is ready to land, you can apply an automerge:* label to instruct Mergify to "merge" the PR once all conditions are satisfied. The conditions are as follow:

  • One approving reviewer (with write permissions, and satisfying any code owner rule), no request for changes.
  • All required checks pass (denotated as "Required" in the list of checks, except for the "Merge strategy (chosen)" check, see below)

The following merge modes are available:

  • automerge:squash - Merge the target branch into the PR branch (if not up to date), then squashes all commits of the PR into a single commit applied directly to the target branch.
  • automerge:rebase - Rebase all commits (with autosquash for commits with fixup!-style messages) onto the target branch, then create a merge commit of this rebased branch onto the target branch.
  • automerge:no-update - Merge the PR branch as-is with a merge commit into the target branch. Requires the PR branch to be up-to-date with the target branch, and that the PR branch has no merge commits. This is an expert option which will fail if there are any other PRs preceding it in the merge train.

The recommended mode depends on the use case and individual constraints:

  • no-update if you must keep the original commits intact, such as carrying signed commits or merging a branch containing tags (like a release branch)
  • squash if the PR started as a single commit, or is meant to be a single, small, atomic change
  • rebase for all other cases (considered the default)

Merging with high priority

If a PR needs to be merged quickly, you can use the priority:high label to assign it a higher priority than other PRs. Higher priority PRs preempt lower priority ones, and therefore are merged first.

Linear history

We attempt to keep a non overlapping "merge linear" history, and checks will enforce this. Unlike Github's strict linear history which does not allow merge commits at all, we allow a single merge commit for landing an up-to-date PR. That allows keeping small individual commits grouped in a larger atomic change to the target branch, while avoiding unrelated overlapping changes.

If for some reason you need to allow a merge commit inside your PR, you can apply the bypass:linear-history label to your PR. This label only has an effect on PRs merged manually or through automerge:no-update.

Note: this label will also bypass the check for the absence of any fixup! or squash! commits that would otherwise be prohibited in no-update PRs.

Bypass Mergify

If for some reason the PR cannot be merged through Mergify, for example it contains some mergify config changes, the label bypass:automerge can be applied to the PR. It will satisfy the required check for an automerge strategy to be chosen, and allow the PR to be merged using the GitHub "Merge" UI button.

Pull Requests, CI and Mergify process

Review process

All GitHub actions (CI jobs) run on a simulated merge of the PR branch onto the target branch. If there are merge conflicts, the actions do not run, and merge conflicts must first be manually resolved (e.g. merge the target branch locally into the PR branch, resolve conflicts, and push the branch).

During the PR review process (once the PR is no longer draft and reviewers are assigned), there is usually no need to keep the branch up-to-date with the target (CI checks run on a simulated merge). If for some reason the PR must be updated while reviews are in progress, favor updating the branch by merging the target into the PR branch. GitHub UI can elide merge commits making it obvious where changes occurred.

Avoid rebasing the PR while reviews are in progress as it makes it very difficult to understand what changes happen during a force-push, and GitHub can lose track of where PR comments belong. If you must rebase the PR while reviews are in progress, coordinate with reviewers first. Prefer first updating the PR branch with a merge of the target branch, then rebase the branch onto that merged commit so that the force-push of the rebase introduces no changes.

Merge process

Once all conditions are satisfied, aka once the PR is approved and all required status checks pass, Mergify will add the PR to the merge queue. One of the required checks is that a merge strategy is chosen, i.e. an automerge:* label is applied. This means that until the label is applied, the GitHub UI will show that some checks are pending, and that the PR cannot be merged. This is to avoid triggering a manual merge by mistake.

Once the PR is at the top of the merge queue, Mergify will update the PR according to the merge mode (rebase or merge), re-run required checks if needed, run integration tests, then merge the PR following the mode requested (squash or merge).

Integration tests are CI jobs that run only once a PR is ready for merge (up-to-date and with an automerge:* label applied). They are more expensive than other CI jobs, and fail less often so we avoid running them for every push. If a PR may potentially cause the integration tests to fail, the author can apply the force:integration label to run these tests with every PR push.

If you'd like, you can see the current merge queue at https://dashboard.mergify.io

Protobuf / Breakage

One of our CI tests verifies that there have been no breaking changes in the Protobuf .proto files. If you have made a breaking change but still want the PR to be merged, add a proto:expect-breakage label to it, in addition to the above automerge:* label. Then Mergify will only land the PR if there was a breaking proto change. If there really wasn't a breaking change, remove the proto:expect-breakage label; you don't need it.

Filter email notifications

Our CI jobs are configured to be cancelled if still in progress and a new run is triggered (e.g. by a new push). Unfortunately, GitHub sends an email every time Actions jobs are cancelled. To avoid cluttering your inbox you can create a GMail filter:

subject:("Run Cancelled") list:(github.com) cc:([email protected])

With action "Skip inbox, Mark as read"

Clone this wiki locally