Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support squash merges #25

Open
pitaj opened this issue Jun 11, 2023 · 11 comments
Open

Support squash merges #25

pitaj opened this issue Jun 11, 2023 · 11 comments

Comments

@pitaj
Copy link

pitaj commented Jun 11, 2023

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
Copy link
Member

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
Copy link
Author

pitaj commented Jun 11, 2023

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
Copy link
Member

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

@pitaj
Copy link
Author

pitaj commented Jun 11, 2023

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
Copy link
Contributor

Kobzol commented Jun 13, 2023

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
Copy link
Author

pitaj commented Jun 13, 2023

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
Copy link
Contributor

Kobzol commented Jun 13, 2023

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
Copy link
Author

pitaj commented Jun 13, 2023

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
Copy link
Author

pitaj commented Jun 13, 2023

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

@Kobzol
Copy link
Contributor

Kobzol commented Jun 13, 2023

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

@tgross35
Copy link

tgross35 commented Jul 8, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants