Migrating to GitHub's merge queue

[thomaseizinger]

At the moment, we utilize mergify to automatically merge PRs without having to baby-sit them. This incurs some complexity in the form of configuration (https://github.com/libp2p/rust-libp2p/blob/master/.github/mergify.yml), documentation (https://github.com/libp2p/rust-libp2p/blob/master/docs/maintainer-handbook.md#mergify and https://github.com/libp2p/rust-libp2p/blob/master/CONTRIBUTING.md#merging-of-prs-is-automated) as well as conversations with contributors where we educate them about this.

GitHub also has a native merge queue feature which would solve some of these problems because at least some users are likely familiar with the feature.

There is only one caveat: Mergify allows us to customize the commit message that is generated upon merge. See

rust-libp2p/.github/mergify.yml

Lines 5 to 12 in 980bda0

	commit_message_template: |
	{{ title }}
	{{ body | get_section("## Description", "") }}
	Pull-Request: #{{ number }}.
	{{ body | get_section("## Attributions", "") }}

. This is a really nice feature because it means we can edit the PR description ahead of time and know that the commit message will be correctly formatted.

But, this also comes with some complexity. The templating language is limited and to be fully compliant with git, we should actually not have any newlines between trailers¹. One of the trailers that we automatically add via this templating mechanism is the pull-request number. By default, this would be included in the title but that would take away several characters from the allowed 72 chars of a commit message title.

The only configuration that GitHub allows for customizing the commit message is to default to the PR title and description. I would not expect GitHub to implement anything else any time soon, if ever. It is usually not their style to implement detailed configuration options the way e.g. mergify has them.

Overall, I'd like to move to GitHub's merge queue but I'd also like to retain (and ideally improve on!) the commit message of our merge commits. I am gonna create a separate thread for each point below.

Footnotes

1. Trailers are the lines at the end of a commit that say things like Resolves: #1234, Co-authored-by: Foo Bar <[email protected]> etc. ↩

[thomaseizinger]

Remove PR template

At the moment, we have a PR template that contains several markdown sections including a checklist on what users should do as part of their PR. This checklist should obviously(?) not land in the final commit message so we would have to remove it from the description / template.

@mxinden: I remember you saying that you added this to direct users towards what we'd like them to do as part of contributing. Since then, we've also added a CONTRIBUTING.md though which explains (most) of these aspects. Personally, I never look at / enforce this checklist. Are you open to removing this in favor of an overall simpler process of merging PRs?

[mxinden]

Yes

[thomaseizinger]

Referencing of pull-request number

I think it is important to link from the final commit back to the pull-request. When using git-bisect or git-blame, one only has the commit message available and being able to easily navigate to the PR can lead to valuable information.

With GitHub's merge queue, we don't have the ability to automatically add this. I see two options for how we can enforce that is there:

1. Create a CI lint that analyses the PR description and makes sure that Pull-Request: #1234. is there.
2. Add a CI action that automatically adds this reference at the end if it is not present.

[mxinden]

I am surprised. Does it not add the PR number like the good-old "Merge" button does?

[thomaseizinger]

I just did some research and yes it does include it, albeit in the title and not as a trailer in the body. I could live with that for now. It would be good to adapt our lint to exclude those from the allowed characters to not break the title unnecessarily when rendering the commit.

[thomaseizinger]

Submitted some feedback to GitHub: https://github.com/orgs/community/discussions/30439#discussioncomment-7246436

[thomaseizinger]

To fix our semantic PR lint, we will have bring https://github.com/pl-strflt/.github/blob/main/.github/workflows/reusable-semantic-pull-request.yml back into the repository because I'd like to account for the added (#XXXX) in the commit message title.

[thomaseizinger]

Linting of pull request description

In general, I think we would have to create a CI lint to enforce that the PR description has a certain format. For example, that there are no new-lines between trailers, that the linked issues exist, no HTML comments, etc.

[thomaseizinger]

Just noticed that GItHub's merge queue seems to automatically add a contribution notice. See the following commit: firezone/firezone@3d3e737

In there, I had added the Co-authored-by tag manually in the PR description which turned out to be unnecessary because it is already added by GitHub below the -------- line.

cc @jxs This might be help if you want to migrate away from mergify.