How to maintain the squash commit message

[git-hulk]

As @enjoy-binbin mentioned in #1546 (comment), the top commit message is very important to retrospect the background and motivate. @ShooterIT is also familiar with the Redis community and also proposed this before, but we didn't document it.

So I think we can follow the Redis way, how do you think? @PragmaTwice @torwig @tisonkun @xiaobiaozhao.

[mapleFU]

Arrow has a script for that: https://github.com/apache/arrow/blob/main/dev/merge_arrow_pr.py#L588-L635
Maybe we can use something similiar.

[enjoy-binbin]

I think maintaining this commit message is what maintainers need to do.
Although everyone's style is different, it feels weird to use a automated script to do this.
If we maintain the top comment, the final squash message is very easy to use.

[mapleFU]

1. It requires the patching has a template
2. It using the comment of the template to generate the final description

I guess it's not weird because the pr template would be like:

## motivation

<---
describe what you want to do
--->

The final output would be:

## motivation

(your input)

The script is used to strip these comment, not to merge users comment and generate message

[enjoy-binbin]

ohh i got it. Thanks. At least when the pr is merge, the maintainer need to double check the message

[enjoy-binbin]

thanks for the pushing, here are the Contribution Guidelines for Redis: redis/redis#11127

This section (Pull request's commits, top comment, and merging) inside has more practice with Redis

[enjoy-binbin]

for a better views, i copy it's section:

Pull request's commits, top comment, and merging

We aim for the PR top comment to represent a comprehensive list of changes, implications, side effects, design consideration, refactoring and other unrelated changes, as well as any other important information that can be useful instead of reading the actual changes.
For bug fixes, it is important to state the scenario which leads to the bug, and the implication on when it happens.
We usually aim to merge PRs with a squash merge, creating one commit in the target branch, and use the PR top comment as the commit comment for the squash commit.

Because of this squash merge strategy, it is often preferred to avoid amending commits and force-pushing, and instead keep adding incremental changes as separate commits, this makes it easier to review.

There are cases where we'd like to avoid using the squash merge:

1. On PRs that have several distinct topics, each standing for itself in a separate commit, we can just merge these with a rebase merge.
2. On PRs that contain massive refactoring, we usually ask for the renames and code relocation to be done in separate commits from the actual changes (these commit don't actually stand for themselves or pass compilation), for these PRs we'll use a normal merge, creating a merge commit and posting the top comment as the merge commit's commit comment.

On PRs that are marked with release-notes, specifically ones with a lot of details and a long top comment, we would also like a section in the to comment containing a short release note statement (or statements if there are several of them)

[git-hulk]

@enjoy-binbin @mapleFU Thanks for your awesome input.

[tisonkun]

My practice in our project is -

1. Retain sign-off and co-author-by footers.
2. Drop meaningless (intermediate) commit messages when the author didn't prepare it.
3. Retain commit messages when the author carefully write it.

It's easy for me to follow. Any "principle" or "best practice" proposal can work to a better situation, but it shouldn't be a blocker to contributors - I'm fine with contributions without better written commit message, as long as the PR description is clear or the change is itself straightforward.

[tisonkun]

Often, I modify the commit title to follow conventional commit when I have a good idea to rewrite it. It doesn't cause any complain so far. I don't think we should set up a complicated rule on doing so but practice it and conclude what we do (that is, no foreseeing rules). I believe the Redis community summarizes their practice instead of presetting rules.

[git-hulk]

Yes, it's not a blocker for contributors but requires the maintainer(merge operator) to take care of the summary commit message. And it should be a good practice instead of a rule, the maintainer can help to summarize the change if the commit message is not clear enough to explain what it does. Of course, it needs to do nothing if the contributor does well with the commit message or the change itself is straightforward.

[enjoy-binbin]

So do we have an agreement or a consensus on this?
Maintain changes in a top comment for a PR, and using the PR top comment as the squash merge commit message?
Of course, it is not necessary to copy all top comments, but make sure that the commit body clearly reflects the changes or the ideas.

[git-hulk]

It's yes since no one objects. I try to recap your proposal, can help to correct me if I was wrong.

• If the top comment is clear enough, bring it as the squash merge commit message
• If there's no top comment:
  • The PR subject cannot exactly tell what it's modified, the maintainer helps to write or ask the contributor to help if not sure.
  • The PR subject can tell what it does, no top comment is fine.

[enjoy-binbin]

yean, LGTM