Include Merge Commits in Commit Guidelines

[Kissaki]

Our Commit Guidelines in COMMIT_GUIDELINES.md https://github.com/mumble-voip/mumble/blob/master/COMMIT_GUIDELINES.md guide for individual change commits, but not merge commits.

Current Practice

Currently, it seems like we have a mixture of merge commit titling

• Merge PR #<id>: <MR/change title>
• Merge pull request #<id>: <MR/change title>
• Merge pull request #<id> from <thirdparty/branch>

Suggestion 1: Simple, unquestionable definition

Consolidating current practice into a defined rule of preference is simple, and doable without much discussion.

1. Define that Merge PR #<id>: <change title>
2. <change title> must conform to change commit guidelines

Suggestion 2: More elaborate uniformation; changes current practice

Define that

1. Merge Request describes a changeset [of one or more commits]
2. MR titles must be titled according to commit guidelines
3. A merge commit shall be the MR title + description, with the title appended with a (PR #<id>) suffix

<MR title> (PR #<id>)

<MR desc>

---

Reasoning

I would prefer

1. Consistent merge commit naming
2. Concise PR over pull request
3. Concise (PR #<id>) suffix over Merge PR #<id>: prefix [1]
4. Merge commit follows commit conventions/guidelines - Merge commit has same form as other commits
5. MR title becomes merge commit title + MR id ref suffix (PR #<id>), and MR description becomes merge commit description

[1]: Not only is it more concise, IMO it is also more consistent in what commits represent. An individual change commit title describes the change. Why does a merge commit describe a git operation?

My work-preference, which I have good experiences with, is a merge commit representing the entire changeset of the one or multiple commits, or equivalent to what the GitHub merge request is and does.

In cases where the MR consists of only one commit, the merge request and merge commit essentially duplicate information. Which I think is acceptable. The explicit merge commit clearly, formally indicates that it was/is a MR. A reasonable alternative is allowing merging merge requests as single commits on the target branch, adding a MR reference to the commit title. (I like the conciseness and cleanness of that approach. It evades the information duplication and redundancy at the cost of not seeing tree-blocks in git history for every merge request changeset.)

Generating Changelogs

IIRC we have a script to generate a changelog, also to prepare releases. I don't know if that is indeed and still the case. But I have to assume the workflow is currently lacking because of "dirty" merge request names.

Having conforming merge request names allows generating a clean and fully useful changelog from the first-parent commits (normal commits + merge commits - excluding merged-changeset-commits). Having consistent TYPE allows categorizing them.

---

RFC @Krzmbrzl @davidebeatrici @Hartmnt

[Krzmbrzl]

We indeed have a changelog script and it should still work fine.
And effectively the implicit rule always has been (at least to my understanding) to do

Merge PR xyz: PR title

where a PR's title follows the commit guideline.

Explicitly documenting that seems like a good idea though

[Hartmnt]

We indeed have a changelog script and it should still work fine.

It does not. I had to hotfix the regex for the last release. I just forgot to create a merge request...

[Hartmnt]

By the way, I was always using whatever GitHub pre-populated the text area with. I have no strong feelings on how to format merge commits one way or the other. Is there a way in GitHub to set a default message for this as there is in GitLab?

[Kissaki]

By the way, I was always using whatever GitHub pre-populated the text area with. I have no strong feelings on how to format merge commits one way or the other. Is there a way in GitHub to set a default message for this as there is in GitLab?

In the repo settings the default merge commit fill can be changed.

Not to the same degree as in GitLab (default choice or custom template). But we can set it to use MR title or MR title + MR description.

https://github.blog/changelog/2022-08-23-new-options-for-controlling-the-default-commit-message-when-merging-a-pull-request/#options

[Hartmnt]

In a repo where every commit is a concise "atomic" logical change, I do not care one bit about merge commits. So please someone else have the final word on this.

(I could live with a fast forward only repo ducks and runs away)

[Krzmbrzl]

In a repo where every commit is a concise "atomic" logical change, I do not care one bit about merge commits

That's also true. Still, I feel like for consistency's sake it'd be nice if merge commits would more or less follow the same formatting rules as other commits 🤷