trino
trino copied to clipboard
Detect and block merge commits in CI
Description
Detect and block merge commits and commits with fixup!
in the message.
Is this change a fix, improvement, new feature, refactoring, or other? improvement
Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific) ci
How would you describe this change to a non-technical end user or system administrator? n/a
Related issues, pull requests, and links
Documentation
(x) No documentation is needed. ( ) Sufficient documentation is included in this PR. ( ) Documentation PR is available with #prnumber. ( ) Documentation issue #issuenumber is filed, and can be handled later.
Release notes
(x) No release notes entries required. ( ) Release notes entries required with the following suggested text:
# Section
* Fix some things. ({issue}`issuenumber`)
Yes, we should block fixup commits too. If one of those goes in it’s probably a mistake — the author forgot to squash.
Detect and block merge commits and commits with
fixup!
in the message.
This will make contributors think that fixup commits shouldn't be used, while we actually encourage people to use them, during the review. based on Starburst internal experience, this probaly won't have a positive net effect
let's focus on merge commits for now.
Detect and block merge commits and commits with
fixup!
in the message.This will make contributors think that fixup commits shouldn't be used, while we actually encourage people to use them, during the review. based on Starburst internal experience, this probaly won't have a positive net effect
let's focus on merge commits for now.
I added both behaviors, to fail on merge commits and to request changes on fixup commits. The Github Action adds reviews and it looks like this:
and after rebasing:
Tested in https://github.com/nineinchnick/trino/pull/12
See CI failure. Related.
The two failures don't look related, I think it's ready to go now.