rust-lang / bors

Rust implementation of bors used for various Rust components (e.g. the compiler).
Apache License 2.0
45 stars 18 forks source link

Support squash merges #25

Open pitaj opened 1 year ago

pitaj commented 1 year ago

Currently, we rely on the PR author to squash their commits. This results in an extra review cycle.

But often, they'll just end up squashing their commits into a single commit. In those cases, that extra cycle could be avoided by performing a squash automatically.

This feature would enable @bors r+ squash to automatically squash all commits in the PR into a single commit with a message composed of the PR title and body. The squash would be performed before roll-up if applicable, or otherwise before merge.

albertlarsan68 commented 1 year ago

It should force-push the PR branch with the squashed commit, in order to correctly mark the PR as merged. I think it should error if not possible, mainly when the Allow Edits By Maintainers box is unchecked, if possible at all.

pitaj commented 1 year ago

I don't think that is very important, given most PRs aren't marked as merged as it is today. The solution to that is for GitHub to add a way to "Close as Merged".

I'd prefer just performing the squash on rust-lang-ci.

nvm

Mark-Simulacrum commented 1 year ago

I don't understand, all PRs are marked as merged today? I don't think we have problems there...

pitaj commented 1 year ago

Oh hmm for some reason I thought anything part of a rollup wasn't closed as merged. Not sure why I thought that.

Regardless, GitHub has the ability to squash-merge a PR with the API, so shouldn't it be possible to use that?

Kobzol commented 1 year ago

I haven't found a GitHub API that would allow me to squash without doing the actual merge, we need that for running tests before we merge the PR. The goal for this bot was to avoid local git operations, but maybe we will eventually have to give up on that.

FWIW it's still unclear how (and if) will this bot do merges (will PRs run on rust-lang or rust-lang-ci? will we use GH merge queues? etc.).

pitaj commented 1 year ago

I haven't found a GitHub API that would allow me to squash without doing the actual merge

It's a tad convoluted, but I think bors could open a new PR on rust-lang-ci and immediately squash merge it there.

Kobzol commented 1 year ago

It's a tad convoluted, but I think bors could open a new PR on rust-lang-ci and immediately squash merge it there.

If this was the only way, I would probably go the "offline git" route instead. The bot should be generic (=usable for any repository) and it would be quite annoying if it required another repository to perform one of its features.

pitaj commented 1 year ago

The bot should be generic (=usable for any repository) and it would be quite annoying if it required another repository to perform one of its features.

Yeah unfortunately it's not possible to open multiple PRs for the same branch.

@Mark-Simulacrum can you add "ability to apply merge_method (from Merge a pull request) to Merge a branch" to your agenda for discussion with GitHub?

pitaj commented 1 year ago

Actually there is an even more convoluted way to achieve this:

Kobzol commented 1 year ago

I think that creating a new (dummy) PR just for squashing is a no-go.

tgross35 commented 1 year ago

I think maybe what could be better is a command like r+ needs-rebase which would auto-approve any future pushes as long as the content doesn’t change, the commit count is fewer, and there are no new merge commits. Bors could link to some docs indicating how to squash / reword well.

This way you sidestep figuring out how bors could do it, don’t need rereview, and hopefully wind up with a better commit message than the result of a squash. Auto squashed commit messages still wind up containing the small commit garbage messages, like

Add feature to do X

  • format
  • remove old code
  • fixup