Auto-rebase when merging merge request

mentioned in issue #411 (closed)

Is there an impact on the use case highlighted on the screenshot below?

mentioned in issue #896 (closed)

Reassigned to @regisF

Reassigned to @JobV

Milestone changed to %Backlog

Added ~139512 label

Assignee removed

Changed title: Simplification of the merging process through the UI → Automatic rebase when merging

@DouweM as we would like to have that for 8.13, can you comment on technical feasibility and add weight?

Mentioned in issue #408 (closed)

Added ~126459 label

Milestone changed to %8.13

Reassigned to @pedroms

@pedroms Will you take a look at the UX work on this issue? This issue and #896 (closed) are closely related, so I'm going to assign both to you.

Mentioned in issue #896 (closed)

2. Second problem: voting on a merge requests

The second problem comes with our approval feature, i.e. when a MR requires votes to be merged. At the moment, if the MR has been approved and a rebase occurs after that, the approvals are reset and people have to approve the MR again. It's unnecessary.

This could be tricky. If we're temporarily relaxing the constraint that the branch reference remain the same when considering the branch "approved", we could open ourselves to some security issues. We need to be careful that there isn't any sort of race condition whereby one could squeeze in an extra commit during this window and avoid having to get their changes re-approved. Can we "lock" the ~~repository~~ branch during the rebase action? Or could we do the rebase using SHA references instead of branch names, and then atomically set the branch ref back to the resulting SHA afterword?

For instance, if the sequence were:

# target_branch (ref: 1a2b3c4d) currently marked as approved

$ git checkout target_branch
Switched to branch 'target_branch'

$ git rebase master
First, rewinding head to replay your work on top of it...
Applying: ...

$ git rev_parse target_branch
dd00bfd4ed0052cb2bc2abe9fa9b8aeb9ce33d14

# target_branch (ref: dd00bfd4) marked as approved

One could perform a push and change the branch reference for target_branch right before git checkout or in between git rebase and git rev_parse and inject whatever they want. Now, I'm not sure how gitlab-git does these operations internally, so perhaps the git rebase is more explicit and doesn't require a separate rev_parse to identify the resulting SHA. If my understanding here is a bit naive please correct me.

Also, my questions in #896 could be echoed here. Assuming "remove source branch" is unchecked, is the source branch itself modified during a "rebase-and-merge", or is the rebase done in some temporary space leaving the source branch untouched?

Pinging @yorickpeterse for the performance cost of locking a branch etc...

Performance notes:

If we detect that a rebase is necessary when the page loads, we should mention that we will rebase automatically before merging once the Accept Merge Request button is clicked.

This should be fine as long as the check is performed asynchronously. Doing this in the same request as the one used for loading the page can slow this down too much.

Rebase should happen behind the scene automatically after we click on the Merge button.

If an error occurs during rebase, we go through our new Resolve merge conflicts feature.

If there are no errors during rebase, we apply the merge and go through the normal flow after that.

No problems here, we can for the most part reuse what we already have.

Approval strategy. If the MR has an approval strategy in place: ...

No problems here either.

Regarding branch locking, we have the ability to mark a branch as protected which prevents pushing to it. When hitting the merge button we could mark a branch as protected, do all the things we need to do, then remove the protection. This may not be a bad idea in general because right now I think you can actually push during a merge and if your timing is right sneak in extra commits.

Marking a branch as protected is a cheap operation (= inserting a row into the DB) so it should not pose any performance problems.

What's the second operation, after “Rebasing to target_branch”? Is it simply “Merging into target_branch”?

Shouldn't the “remove source branch” be checked/unchecked by default according to the MR submit option?

Edit: Forget the last question, this is already being catered for.

@pedroms The second operation is the actual merge, yes - if the rebase worked.

Here's the UI proposal for this. @regisF's wireframes are spot on, so there's not much I can contribute to improving them:

Rephrased notice to read “source branch” instead of simply “branch”
Rephrased ongoing operations to “Rebasing…” instead of “Rebasing to master”
Added branch reference formatting for masterbranch
Related controls should be disabled while rebasing and merging (“Remove source branch” and “Modify commit message”)

Can we change “Operation 1 of 2” to “Step 1 of 2”? It's shorter and less “machine-speaking”

Edit: Actually, what if it reads “rebase this branch” instead of “rebase source branch”? And “Remove source branch” reads “Remove this branch”? Too much simplification?

@pedroms I believe source branch and target branch are terms we use everywhere. So we might not want to simplify things here - @axil ?

@pedroms @regisF yeah, always better to be explicit than implicit :)

Can we change “Operation 1 of 2” to “Step 1 of 2”? It's shorter and less “machine-speaking”

and I agree with all the other changes in your list :)

@pedroms so is this final? If yes, we can pass it to front-end.

Assignee removed

@regisF Yep, I think this is final. Let's pass it on to the front-end. I'm un-assigning Pedro from this issue. Thanks!

Added ~481018 and removed ~126459 labels

Added frontend label

Reassigned to @mikegreiling

Assignee removed

There's not much I can do here until we have a ~Backend developer assigned to this feature. The frontend work is pretty light and it will mostly rely on knowing how the backend will asynchronously communicate with the frontend as it progresses. I also think there are a few implementation details that need to be ironed out:

If an error occurs during rebase, we go through our new Resolve merge conflicts feature.

So, the user won't know if they're going to be sent off to resolve rebase conflicts until after they click the button? Is there a way to detect if there will be conflicts before the user attempts to rebase? I prefer how this is handled with merge conflicts, where you are explicitly clicking on "resolve these conflicts" to kick off the feature.

@yorickpeterse could you comment on the feasibility here?
Resolving rebase conflicts is a very different beast compared to resolving merge conflicts, so I don't think we can just drop-in the same code here. Since rebasing entails starting at the head of the target branch and "replaying" each commit from the source branch one at a time, we can potentially run into conflicts with any given commit as we step through each commit in order. The main complication here is that, unlike resolving merge conflicts, this is not all done in a single step. It will require a temporary work state to be maintained in the backend while the rebase is in progress. Every time a conflict occurs, the rebase is paused and the user is prompted to resolve it. We have to consider all of the edge cases here. What happens when a user abandons a rebase in progress? What if they get two conflicts in and decide to abort? What if we get into the type of conflict resolution that simply picking "use yours" or "use theirs" won't fix?
Approval strategy. If the MR has an approval strategy in place:
- Same as now: it's still not possible to click on the Accept Merge Request button if the number of approvals necessary for merging is not reached.
- Same as now: if a user rebases from the CLI and pushes the changes to the MR, we should reset the approvals.
- Different from now: in the UI, when clicking on the Accept Merge Request button, if a rebase is necessary, this rebasing should not reset the approvals.
I think this is fine as long as we don't run into conflicts while rebasing. Once we start making changes to the source branch via rebase conflicts I think approvals should be reset.
We still haven't settled how forked repos are handled. In the case of a forked repo branch, we may have a situation where the owner of the fork has permission to rebase the source but does not have permission to merge into target, and the person trying to merge the branch may have permission to merge into the target branch, but not have permission to rebase the source branch (since that would require having commit access on the fork, which he/she doesn't own). Should the branch be checked out locally into a temporary space to perform the rebase + merge, or should these situations just require resolution the old way?

One way I see to resolve this is akin to the "delete branch after merge" checkbox. When you check this, you're essentially giving the person who merges the branch permission to reach into your fork and delete your branch. Perhaps we can have a checkbox for a similar abdication of branch permissions for rebasing?

@mikegreiling Regarding detection, we may be able to do so but I think this would be a rather expensive process. It's certainly not something you want to run every time somebody views a merge request. There's also the possibility of a conflict occurring during a rebase (that is, after you check the status on a MR page but before you hit the button).

Reassigned to @vsizov

As it was already mentioned the rebase conflicts differ from the merge conflicts. We have nothing to resolve the rebase conflicts with UI so I propose always ask to rebase manually if there are some conflicts.

@vsizov Alright!

@mikegreiling apologies if this was clarified before with Yorick and Valery's comments, but: rebase conflicts are way more complicated to check, because you can have a conflict in a rebase when you wouldn't in a merge. I think if we hit a conflict when rebasing that wasn't detected beforehand, we should just abort and show the manual message.

Assignee removed

What would be cool in future is to do something like git rerere to store those resolutions. But that's way out of scope for this

@regisF Is this shipping in 8.13?

Milestone changed to %8.14

Milestone changed to %Backlog

Removed frontend label

Milestone changed to %8.14

Milestone changed to %Backlog

this feature is featured on the direction page under %8.14 https://about.gitlab.com/direction/

@dimitrieh that's fixed now, but good catch! The direction issues page is generated from the milestones at the deploy stage: https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/generators/direction.rb That means it needed a new deploy of www-gitlab-com to pick this up.

Added ~238305 label

Milestone removed

Added ~874214 ~155976 and removed ~238305 labels

@smcgivern : For auto-rebasing, should we do it for both of these options in one issue? Or does it make sense to do one of them first?

Merge commit with semi-linear history
Fast-forward merge

Changed title: Automatic rebase when merging → Auto-rebase when merging merge request

Added ~1012342 label

Milestone changed to %Backlog

@victorwu both required a rebase AFAIK, so they use pretty much the same code. May as well do them at the same time!

changed milestone to %8.16

removed ~1012342 label

We will have a meeting to hash out the detailed approach of this and https://gitlab.com/gitlab-org/gitlab-ee/issues/150 once we confirm that they are scheduled for 8.16.

mentioned in issue #150 (closed)

added Deliverable label

@victorwu please invite me for that meeting as it may impact the UX. Thanks

@awhildy : Adding @pedroms to the meeting. Feel free to both attend or just one of you. We definitely need UX representation. But don't want to take too much of your time if it's not necessary.

@pedroms - I'll let you cover the meeting next week Let me know if you can't attend. Thanks!

assigned to @smcgivern

We need to remove the part about notifying in the case of failure - @pedroms.
We also shouldn't show the steps as it's a transactional thing, so you're never in an intermediate state.
We will remove the existing two-step process so that this can only ever be performed by people who can accept the MR. This means that an MR can't be rebased to just affect the source branch - we can add this back later if needed.
We will do the same as squashing - this never updates the source branch, only the target branch.
We will perform a rebase-ability check in the case of the last two merge methods (semi-linear and fast-forward), instead of our current merge check. This will cherry-pick the commits in sequence from the source branch to the target branch, and fail if any of those cherry-picks fail. This may be tricky with merge commits. If there is only commit in the MR diff, or we are squashing, we can just do the merge check as now.
We need to express this to the user: if the rebase-ability check fails, you can only accept the MR if you either a) squash or b) rebase manually on your local machine. That means that we'd still show the accept button, but it would only work for the squash.

added ~126459 label

@smcgivern Let me know if this makes sense. The “Learn more” would link to the docs. The instructions in the modal may need some tweaking.

If the rebase-ability check passes	If the rebase-ability check fails	Pressing “rebase locally” shows this modal

Edit: Also, after the squashing checkbox is added per https://gitlab.com/gitlab-org/gitlab-ee/issues/150, it would appear checked and disabled if the rebase-ability check fails.

@pedroms thanks!

The first screenshot looks great!

I don't think the second screenshot is quite right, because it's far too subtle that we will be squashing the commit. In addition to that, that case is only possible if the rebase-ability check fails but the merge check passes. I wonder if we should just show instructions to rebase manually if this check fails, even if the MR is squashable, and leave this for a later iteration. What do you think? /cc @victorwu

The third screenshot also looks good, is that an existing modal or do we need to create a new one based off the merge instructions modal?

@smcgivern Good

Do you consider that squashing commits could be an undesirable action in some scenarios? In other words, do you think that rebasing is considered harmless and doesn't need a warning tone, and squashing can be considered harmful and needs a warning tone? E.g.:

The third screenshot is a new one based off the existing local merge instructions modal.

mentioned in merge request !1024 (merged)

@pedroms I think it's a warning if the 'squash commits' checkbox wasn't already checked. That's why this is confusing, and that's why I'd rather the leave it out of this iteration

@smcgivern Oh, I wasn't considering the “Squash commits” option so that we can separate this issue from https://gitlab.com/gitlab-org/gitlab-ee/issues/150. That's why I didn't include the checkbox just yet.

Anyway, assuming we'll leave this “squashing” scenario for a later iteration, when you say:

show instructions to rebase manually if [the rebase-ability] check fails, even if the MR is squashable

Is the “Accept Merge Request” button available in this scenario? Or is the only option for the user to rebase and push locally?

@pedroms sorry, let me try to clarify!

We currently have a merge check. We want to add a rebase check, because it's possible that an MR can be merged, but cannot be rebased (as one commit adds a conflict that is resolved by a later commit - for instance, if you used our resolve conflicts feature).

So we'll add this rebase check and show it if the MR cannot be rebased before accepting, rather than have the stand-alone rebase button like we do now.

However, if you squash the MR into a single commit, then if the merge check passed, we can accept the MR even if the rebase would fail, because the squash won't have that conflict at an intermediate commit. But you can only accept it if you squash. This is the case you're designing for above.

I think we should just show the not-rebaseable message in this case and try to handle that case in future. Technically, it isn't too complicated, but as you can see, it is potentially very confusing!

@smcgivern Yes, I understand, thank you for explaining. Let's proceed as you suggest and improve it later.

In terms of showing the “not-rebaseable” message, what do you think about something like this:

Since the purpose is to only update the target branch, if the user doesn't have merge access for that branch:

Hey, it's vital to rerun the CI after the rebase and before the merge IMO. Logical conflicts can break the build without having a git conflict and this happened multiple times already for teams I have worked with. One of the main points of a fast-forward only workflow is to have CI already done for the commit that master is fastforwarded to, if you auto rebase and merge it you take this away and make it almost as bad as a merge workflow.

@sils that's a great point! So at the moment, you rebase and then set Merge When Pipeline Succeeds?

I think https://gitlab.com/gitlab-org/gitlab-ce/issues/4176 would help with this.

Yes, rebase, reapprove, merge after pipeline succeeds. I think gitlab already shows the new commits introduced due to the rebase, those are important for the reapproval so we can check wether any of this might logically conflict.

@smcgivern related issue: https://gitlab.com/gitlab-org/gitlab-ce/issues/12505 if a rebase is needed (which is usually a matter of seconds) I'd rather want to do it before wasting time on a human review.

OK, so in that case we definitely shouldn't do this:

We will remove the existing two-step process so that this can only ever be performed by people who can accept the MR. This means that an MR can't be rebased to just affect the source branch - we can add this back later if needed.

i.e. we need to support both workflows. Thanks for commenting, I'm really glad I didn't make this much worse for you.

@sils I think that was a duplicate of https://gitlab.com/gitlab-org/gitlab-ee/issues/372, in which case https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/489 will have fixed it in 8.10. Are you still seeing this issue?

It's not, a rebase should reset the approvals (and you have a setting for that which allows being strict about that or not ). The point of that issue (and maybe this discussion should happen over there if it goes on) is that I should be able to first rebase, then review and approve because the rebase (just as a merge) is a change that can break things. One less review needed.

mentioned in issue #1511

@pedroms @dimitrieh @victorwu given the above, here's what I think we should do:

If a rebase is required and possible, let you do that either in a one-step process with the merge, or as a separate step.
- If you can't merge for some other reason (for instance, you haven't approved the MR), but you need a rebase anyway, we should still show the button and let you do it as a separate step.
If a rebase is required and not possible, we show the error as per the designs.

So the big change here is to make the existing rebase button stay, and actually be present more often. This ties into the MR widget redesign stuff. Any thoughts?

@victorwu @regisF in light of https://gitlab.com/gitlab-org/gitlab-ee/issues/895#note_21371073, I'm setting the milestone on this to 8.17 for now.

changed milestone to %8.17

Thanks @smcgivern . Thanks for getting auto squash through for 8.16. I think at the outset we were pessimistic we would get anything completed for 8.16. So getting squash in is a win!

I'm fine with the proposed design in https://gitlab.com/gitlab-org/gitlab-ee/issues/895#note_21371073 as you've described. This way we actually don't regress in functionality right? We still get to rebase as existing functionality, regardless of the situation.

@victorwu squashing isn't done yet, but it's in review!

You're right, we don't have any loss of functionality here, which is important for the case described at https://gitlab.com/gitlab-org/gitlab-ee/issues/895#note_20948625.

@smcgivern : Yes, forgive my optimism in squashing getting done this week in time. I've been following along in the issue/mr. =p.

mentioned in issue #1603 (closed)

@regisF @victorwu I'm moving this to the backlog pending https://gitlab.com/gitlab-org/gitlab-ee/issues/895#note_21371073. I'm happy to pick this up once we have a decision.

changed milestone to %Backlog

removed assignee

Hey btw. potential corner case that you might have already thought about: if I push that rebase and merge button at two merge requests the need to be rebased sequentially!

mentioned in issue #372 (closed)

@smcgivern from your comment: https://gitlab.com/gitlab-org/gitlab-ee/issues/895#note_21371073

If a rebase is required and possible, let you do that either in a one-step process with the merge, or as a separate step.

I guess that would be something like the image below. Should we also support the case for “Rebase and merge when pipeline succeeds”? Also, who can perform the rebase (I got lost in this discussion)?

If you can't merge for some other reason (for instance, you haven't approved the MR), but you need a rebase anyway, we should still show the button and let you do it as a separate step.

I'm having trouble creating an unapproved MR, but maybe it's just a matter of disabling the Rebase and merge button? Furthermore, we can enhance it with a tooltip so users can understand why it's disabled.

@smcgivern @sils would it make sense to have a project setting to choose between allowing either auto-rebase and merge (1-step) or rebase and then merge (2-steps)? I'm basically trying to simplify the UI so we don't have two buttons. According to @sils's prior comment, the need to rebase and merge separately seems to be a project/team decision. Let me know if I got this wrong

This is more or less correct. We might be able to simplify it to the first case reasonably, if you show the new commits that will be rebased upon nicely. Then I as a reviewer will be able to decide if that rebase will break anything logically and just let the machine do the work, that'd be ideal for both cases I think.

with only one button

added Stretch and removed Deliverable labels

changed milestone to %8.17

Hi,

We're currently looking into ditching Gerrit to due the pain or rebasing+merging multiple commits, I was directed to this issue by your sales people and I think it's on the right track but can be done better. Unless I misunderstood the description in the beginning.

We re-run tests after rebase since commits sometimes are affected by recently merged commits.

As I understand this proposal the workflow would be:

Mark patch p1 for rebase+merge, it's rebased
tests run
p1 is automatically merged
Mark patch p2 for rebase+merge, it's rebased onto p1
tests run
p2 is automatically merged
Mark patch p3 for rebase+merge, it's rebased onto p2
tests run
p3 is automatically merged

But this is what I'd like:

Mark p1, p2, p3 for rebase+merge. Either in one action or three clicks in a row, doesn't matter.
tests for p1 is run and pass
p1 is merged
Attemptes automatic rebase of p2 onto p1, it fails so it's reported as needs manual rebase and is skipped for now
Attempts automatic rebase of p3 onto p1, it succeeds
tests for p3 is run
p3 is merged

Even less manual work :)

@pedroms thanks for that!

I guess that would be something like the image below. Should we also support the case for “Rebase and merge when pipeline succeeds”? Also, who can perform the rebase (I got lost in this discussion)?

Yes, definitely. I assume we'd use the same dropdown button as we do elsewhere, although that's a really long title. There is an ordering problem here, too, which I'll get to in a bit - see https://gitlab.com/gitlab-org/gitlab-ee/issues/1230.
Anyone who can push to the source branch can perform a rebase.

I'm having trouble creating an unapproved MR, but maybe it's just a matter of disabling the Rebase and merge button? Furthermore, we can enhance it with a tooltip so users can understand why it's disabled.

You need to enable approvals in project settings, but I think there was a bug about the ordering of those options that is now fixed in master. I think your design makes sense, but it does go away from the 'one primary action' available on an MR that we have at the moment.

@mikael.modin.foreseeti thanks for your thoughts! I think we have a real conflict here between two types of user, or workflow:

Those who, once a merge request is ready, just want to get it in the target branch as soon as possible.
Those who, once a merge request is reviewed, want accepting it into the target branch to run through all the checks in order, so that they can be as confident is possible that this change is what they want.

(This is a slight tangent, but I think this is a way to think of https://gitlab.com/gitlab-org/gitlab-ce/issues/21947 - some people want to resolve conflicts while merging, while others want to resolve conflicts and then review, run tests, etc. again.)

That's a long-winded way of saying that I think your proposal is reasonable, but I think other people will disagree with it

One problem I do see is with the rebase + merge when pipeline succeeds case. In your second example, what would happen if between 6 and 7, the p3 needed another rebase?

Would we rebase again, and run the pipeline again, and set merge when pipeline succeeds again?
Would we just fail to merge, and create a todo for the author of the MR and the user who set MWPS?
Would we rebase again, but not run the pipeline again, and just merge immediately?

I think a lot of people will be expecting 3, but I agree with you that that has negative implications for correctness. I wonder if 2 is the best compromise here. What do you think? cc @victorwu

mentioned in issue #1230

@pedroms we're leaning towards this for the 'rebase and merge when pipeline succeeds' case:

All of these steps are async, but happen in sequence.
Rebase the MR.
Wait for a pipeline on the new HEAD commit.
Set the MR to merge when pipeline succeeds.
Wait for the pipeline to finish.
If it succeeds:
1. If the MR is still FF-able, perform a fast-forward and close the MR.
2. If the MR is not FF-able, create a todo and cancel the merge when pipeline succeeds status.
If it fails, create a todo but don't cancel the merge when pipeline succeeds status.

How does that sound?

changed milestone to %9.0

After chatting with @victorwu, this issue has changed to %9.0.

@smcgivern

Anyone who can push to the source branch can perform a rebase.

I'm a bit confused as to who can do what. Initially, I thought we were going to do it so that only people that have merge access to the target branch could do the rebase+merge. But maybe I'm just mixing the concepts, so please feel free to direct me on this.

My perception right now in terms of permissions is:

If the user has access to both the source and target branches, he has the option to Merge or MWPS, and we show a message similar to: “A rebase is required An automatic rebase will happen when merging. To do it separately, rebase only”
If the user only has access to the target branch, he has the option to Merge or MWPS and we show a message similar to: “A rebase is required An automatic rebase will happen when merging. To rebase only, ask @MR_author to rebase the source branch onto the target branch.”
If the user only has access to the source branch, he has the option to Rebase and we show a message similar to: “A rebase is required To allowed this request to be merged, rebase the source branch onto the target branch”

If the MR is not FF-able, create a todo and cancel the merge when pipeline succeeds status.

In this case, the options that are shown would be the ones detailed before, correct?

@mikael.modin.foreseeti thanks for your thoughts! I think we have a real conflict here between two types of user, or workflow:

Those who, once a merge request is ready, just want to get it in the target branch as soon as possible.
Those who, once a merge request is reviewed, want accepting it into the target branch to run through all the checks in order, so that they can be as confident is possible that this change is what they want.

Ah yes, that's two different ways of working and we're more of the second type.

(This is a slight tangent, but I think this is a way to think of gitlab-ce#21947 - some people want to resolve conflicts while merging, while others want to resolve conflicts and then review, run tests, etc. again.)

That's a long-winded way of saying that I think your proposal is reasonable, but I think other people will disagree with it

Fair enough, I accept their right to have the wrong opinion ;)

A bit more serious, should I consider this to be more of the first usage case? (get it in the target branch asap) I don't see a good way of reconciling the differences between the two workflows so they might need slightly different UI. Or am I missing something?

One problem I do see is with the rebase + merge when pipeline succeeds case. In your second example, what would happen if between 6 and 7, the p3 needed another rebase?
Would we rebase again, and run the pipeline again, and set merge when pipeline succeeds again?
Would we just fail to merge, and create a todo for the author of the MR and the user who set MWPS?
Would we rebase again, but not run the pipeline again, and just merge immediately?
I think a lot of people will be expecting 3, but I agree with you that that has negative implications for correctness. I wonder if 2 is the best compromise here. What do you think?

My main point is to eliminate manual steps so 1 would be best, but 3 would be acceptable.

The way I envision this is a pipeline, so when one commit is going through rebase-tests-merge the next(say p4) won't start until p3 has been merged, so there should not be a rebase needed between 6 and 7.

@pedroms that's an excellent point, and one I'd completely missed. Indeed, you need access to both the source and the target branch to be able to rebase and merge. I think your options are correct to what we'd discussed before, but the 'pipeline success' part is a small problem - we'll need a temporary hidden ref for this on the backend, to keep the rebased HEAD that is building a pipeline, but then we wouldn't show the correct status in the MR.

So, contrary to what I said before, I think we should only allow rebase and merge for people with access to both branches. If you have only access to the source branch, you can rebase but not merge. If you have only access to the target branch, you can merge but not rebase. That is the safest option. (This is also the reason I haven't updated for a SSOT - I'm sorry about that, but this is more complicated than I thought, and I want to think it through.)

In this case, the options that are shown would be the ones detailed before, correct?

Correct!

@mikael.modin.foreseeti I'm leaning towards my second option there - if the MR cannot be merged because the pipeline failed or it needs another rebase, just fail and create a todo for the author and the person who clicked the merge button.

This is because it's the most conservative option, and I think both groups of people will be least-unhappy with it

@smcgivern great, thank you for going through this again. And I agree with you, this is not simple and gets confusing sometimes.

Ok, let's keep this focused on the ideal scenario for rebase and merge: you must have access to both branches. That said, does it make sense to create an issue for a future iteration of this feature where we would solve rebase and merge if you only have access to the target branch? (Just making sure we don't lose anything. Feel free to say no.)

If you have only access to the target branch, you can merge but not rebase. — @smcgivern

So if a rebase is required, the user must ask the MR author (or anyone else with access to the source branch) to rebase. Only after that can the merge proceed — correct?

@pedroms your last paragraph is correct, and I believe that this maps to how this feature works already

That said, does it make sense to create an issue for a future iteration of this feature where we would solve rebase and merge if you only have access to the target branch? (Just making sure we don't lose anything. Feel free to say no.)

I think we should wait for people to request it. For people using this workflow, it might not be an issue!

removed ~481018 label

Edited description: clarified cases when user has or doesn't have access to source or target branches; added mockups for applicable cases.

@smcgivern @victorwu In the updated description I've added a few mockups with the following message:

To allow this request to be merged, ask mr-author to rebase it

The idea here was to be specific as to what person you can/should contact. However, there may be other people who have access to the source branch, besides the MR author. Do you prefer the above example or the following?

To allow this request to be merged, ask someone with write access to the source branch to rebase it

@pedroms I prefer the second. Don't forget that even an MR author might not have access to the source branch, just the ability to create MRs in the project! (You can create an MR between any two branches.)

@victorwu I think this should be a Deliverable when we do it, and not be on a release otherwise. Wdyt?

removed milestone

@smcgivern : You mean you removed it from 9.0 right? Okay then let's focus on other stretch issues in 9.0 when we get to them. Tentatively let's schedule it for 9.1 deliverable.

added Deliverable and removed Stretch labels

@pedroms : Yep, agree with @smcgivern above.

@smcgivern @victorwu great, thanks for the feedback. I've updated the linked images in the description accordingly.

mentioned in issue #1822

added ~897283 label

changed milestone to %Backlog

@pedroms I didn't read through this entire thread but it looks like the ux might be done here? Can we move to UX ready or is there still more to work on?

@tauriedavis you're right, this is indeed UX ready. Thanks for noticing!

@victorwu assigning it to you for scheduling.

added ~1268427 and removed ~126459 labels

assigned to @victorwu

changed milestone to %Next 2-3 months

changed milestone to %Backlog

@victorwu Eek. I'm disappointed the 'next 2-3 months' milestone has vanished from this issue. Do you think this could be reconsidered please?

I can provide some more background; this is a really frustrating issue in GitlabEE - the problem is not that we have to press the rebase button and then the merge button. The problem is that we have to press 'merge', then wait 'a while' (sometimes minutes). Sometimes on completion of the rebase the screen updates to say 'The branch head has changed, you need to reload the page'. Sometimes that doesn't happen, so we have to optimistically hit reload a few times until the rebase has actually finished. Sometimes the engineer gets distracted during this process and comes back to find no matter how much they reload gitlab still says a rebase is required - either someone else got a commit merged in the meantime (in which case back to the start of the process) or the rebase failed (in which case gitlab doesn't actually let you that or which file was the issue). Then, if your luck is in, you can actually hit the 'Merge' button.

I can't understate how much smoother this process would be if the button was 'Rebase & merge'!

mentioned in issue #2970

cc @JobV for scheduling since @victorwu is OOO

mentioned in issue #85 (closed)

changed milestone to %Next 3-6 months

This has been really frustrating for our team lately. Even though we only have two developers on our current project, is still a pain. Every time a MR is merged, every open MR needs to be rebased. This then kicks off our Jenkins pipeline flow again.

mentioned in issue #3033

Hitting us hard also on a busy repo.

marked this issue as related to #1511

changed the description

Edited description: moved original description into a <details> tag

Auto-rebase when merging merge request

Resources

Problem

Solution

Original description (with some images removed to save space)

Specification

Documentation

References

Designs

Child items ...

Activity

Admin message

Admin message

Auto-rebase when merging merge request

Resources

Problem

Solution

Original description (with some images removed to save space)

Specification

Documentation

References

Relates to

Activity