Simplify merge policy / conventional commit messages

29 views
Skip to first unread message

Weston Pace

unread,
Dec 4, 2022, 6:50:26 PM12/4/22
to Substrait
We can simplify the merge process so that we do not have to care about
the content of PR commit messages. This makes it easier for those who
are unfamiliar with conventional commits or sophisticated git usage
(e.g. rebasing and squashing into a single commit message). It also
makes it possible to make incremental changes to a PR without
squashing-and-merging everything back to one commit and without
cluttering the commit log.

The default github behavior for squash-and-merge is to use the PR
title if there is more than one commit and the commit message when
there is only one commit[1]. This is historically why we have linted
commit messages as they might be used as the message for the
squash-and-merge entry. However, this is now configurable (as of May)
and we can change it to always use the PR title and description[2].

There is an action we can use to verify the PR title follows
conventional commits[3]. It appears to be configurable enough to
match our current policy. Our current process also checks for
breaking changes with buf and requires a notice in the commit message.
We could modify that check to require a notice in the PR description
(this check is already a custom script).

I could be overlooking something but at the moment I am not seeing any
real downside. If there are no concerns I'd be willing to do the
legwork to switch over the PR checks (though always happy if someone
wants to volunteer ;)

[1] https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/configuring-commit-squashing-for-pull-requests
[2] https://github.blog/changelog/2022-05-11-default-to-pr-titles-for-squash-merge-commit-messages/
[3] https://github.com/amannn/action-semantic-pull-request

Jacques Nadeau

unread,
Dec 4, 2022, 7:31:33 PM12/4/22
to subs...@googlegroups.com
Sounds good to me. It also means people can fix commit messages without having to push new changes. Anything that lowers annoying new contributors with project-specific friction is a win in my book.

We just need to ensure as mergers that we continue to not mess things up (same as now). :D

--
You received this message because you are subscribed to the Google Groups "substrait" group.
To unsubscribe from this group and stop receiving emails from it, send an email to substrait+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/substrait/CAE4AYb2LKCjeanCUGuAhtbxNgsjs%2BV%3DTW_as6vUQU8GS_TBL%3Dw%40mail.gmail.com.

Phillip Cloud

unread,
Dec 4, 2022, 8:13:38 PM12/4/22
to subs...@googlegroups.com
This sounds like a good idea to me as well.

Vibhatha Abeykoon

unread,
Dec 4, 2022, 9:14:34 PM12/4/22
to subs...@googlegroups.com
+1 for this change. I am happy to help out with this effort.

With Regards, 
Vibhatha Abeykoon, PhD


Carlo Aldo Curino

unread,
Dec 5, 2022, 12:21:37 PM12/5/22
to subs...@googlegroups.com
+1 as well seems very reasonable 

Weston Pace

unread,
Jan 13, 2023, 7:27:47 PM1/13/23
to subs...@googlegroups.com
This is ready for review (Matthijs already gave it a review but it
needs a review from someone with write perms):
https://github.com/substrait-io/substrait/pull/410
> To view this discussion on the web visit https://groups.google.com/d/msgid/substrait/CAMreUaxPhyqK1ODWsAZ8M070B-eHt%3DWm3M4krwUu91FjqZZ60A%40mail.gmail.com.

Vibhatha Abeykoon

unread,
Jan 13, 2023, 7:32:36 PM1/13/23
to subs...@googlegroups.com
Weston, I added a suggestion, and reviewed.

--
Vibhatha Abeykoon
Reply all
Reply to author
Forward
0 new messages