-
Notifications
You must be signed in to change notification settings - Fork 731
feat(gitlab): support rebase / squash merge method for MR #11611
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
base: master
Are you sure you want to change the base?
Conversation
|
@nshcr is attempting to deploy a commit to the GitButler Team on Vercel. A member of the Team first needs to authorize it. |
| await api.MergeRequests.rebase(upstreamProjectId, number, { | ||
| skipCI: false | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the rebase operation skip CI? I'm a bit unsure here, since CI will definitely run again during the second merge step.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this was skipCI: true, would that mean that after the merge, CI will run, to possibly detect that something went wrong when it's too late?
It's interesting as what CI should check is what would end up in the target branch if the desired strategy (i.e. merge, rebase, squash) is applied.
Because if skipCI: false then I am afraid that the rebase can't typically complete in time at least if CI needs to be completed for the rebase to be done.
Can this be, that the rebase_in_progress flag is true even after the rebase was done just because CI is running?
Somehow I feel quite uneasy about this manual implementation of a feature that is best implemented/queued in the forge itself.
| const maxAttempts = 30; // 30 seconds timeout | ||
| let attempt = 0; | ||
| let rebaseComplete = false; | ||
|
|
||
| while (attempt < maxAttempts && !rebaseComplete) { | ||
| await sleep(1000); | ||
|
|
||
| const mr = await api.MergeRequests.show(upstreamProjectId, number, { | ||
| includeRebaseInProgress: true | ||
| }); | ||
| // Check if rebase is complete | ||
| if (!mr.rebase_in_progress) { | ||
| rebaseComplete = true; | ||
| } | ||
| attempt++; | ||
| } | ||
|
|
||
| if (!rebaseComplete) { | ||
| throw new Error('Rebase operation timed out. Please try merging again later.'); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is a 30-second polling interval reasonable? If the GitLab pipeline includes long-running CI jobs, this might cause the merge process to time out or be interrupted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me it seems unreasonable to be forced to deal with that as a UI, as the only correct way to do it would be to have no timeout at all. And if that was the case, one would also have to make sure the operation is resumable, and then it gets complicated quickly.
Even with a 30 second timeout I wonder what happens if the user clicks to the branch view and back to the workspace. Will the UI elements still have the correct state, or will the UI be able to race against itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds support for GitLab's rebase and squash merge methods when merging merge requests. Previously, GitButler only supported direct merge operations for GitLab MRs.
Key Changes:
- Implements conditional logic to handle different merge methods (rebase, squash, and merge)
- Adds a two-step asynchronous rebase process with polling for GitLab's rebase operation
- Passes appropriate parameters to the GitLab API for squash merges
| // After rebase completes, perform the merge | ||
| await api.MergeRequests.merge(upstreamProjectId, number, { | ||
| shouldRemoveSourceBranch: true | ||
| }); | ||
| } else { | ||
| // For 'merge' and 'squash' methods, use the merge API directly | ||
| await api.MergeRequests.merge(upstreamProjectId, number, { | ||
| squash: method === 'squash', | ||
| shouldRemoveSourceBranch: true | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on krlvi's reply in this PR (#8222 (comment)), MRs created by GitButler explicitly specify deletion of the source branch, but this isn't necessarily true for MRs created elsewhere.
To ensure the stack branches feature works correctly, I think it's necessary to explicitly pass the shouldRemoveSourceBranch parameter when performing the merge.
eab31b8 to
edb2b84
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
|
Thanks a lot for giving this a shot, and for highlighting possible concerns. For now, I only looked at those and added my thoughts to them. My fear is that due to the way the GitLab API has to be used, implementing the |
Hi! While using the GitLab integration, I noticed that the rebase and squash merge methods don't work.
After checking the relevant code, I found that GitButler only implements logic for direct merges and does not handle other merge methods.
I briefly reviewed the GitLab API documentation, then with the help of Claude Sonnet 4.5, I implemented support for rebase and squash when merging MRs.
All the code has gone through my own review and testing, and both the rebase and squash methods work correctly on the official GitLab instance.
Here are a few differences compared to GitHub that are worth noting:
For more details, please refer to the code comments I added.
And in advance, Merry Christmas🎄! This isn't an urgent request — feel free to leave this PR until after the holidays.
(Sorry for the delayed reply on Discord — the past two weeks have been unusually busy for me. I'll get back to it soon.)