Discussion: Removing the MR bottlenecks which prevents merging
I'm not happy with the timespan it takes for us from creating an MR to the merging of the MR. And therefore i did some thinking and now also write that down!
Codeowners vs required approvals
Problem:
We have three codeowners (Reynald, TJ and TB) but by default the MRs require two approvals (one codeowner and one from a random person). The ratio of required approvals vs people who want to do code review is way too high.
Suggested solutions:
- Add two more code owners (
git shortlog -s 9.3.3.. | sort -n
suggests that Damien und TI would be candiates) - Default to 1 approval. And make it required for the more difficult MRs to have 2 approvals.
Merging with semi-linear history and gitlab bugs
We restrict the potential approvers with the following settings:
Prevent approvals by users who add commits
Prevent approval by author
Which I think is good as there is little point in approving your own work.
The problem is that we also have set Merge commit with semi-linear history
which says
Merging is only allowed when the source branch is up-to-date with its target.
When semi-linear merge is not possible, the user is given the option to rebase.
This requires that each MR is rebased against latest main before it can be merged.
Problem:
-
Requiring a rebase for each MR before it can be merged prevents automatic merge of multiple MRs.
Imagine you have three MRs so that the following is at the moment the required steps:
- Reviewers have finally approved!
- You choose one MR, select "Merge when pipeline succeeds"
- You wait
- First MR merged
- You rebase the second MR and select "Merge when pipeline succeeds"
- You wait
- Second MR merged
- You rebase the third MR and select "Merge when pipeline succeeds"
- You wait
- Third MR merged
That sounds tedious? But not enough or? Well due to gitlab folks not shipping well thoughtout software, you might also find out that your own approvals from the MRs were removed due to the rebase. This is due to gitlab-org/gitlab#337888 (closed) and gitlab-org/gitlab#216144 (closed). And now you need to either bend the rules (which only admins can) and merge, or find another person to add a review.
Solution:
If we would use Merge commit
which says
Every merge creates a merge commit
instead of Merge commit with semi-linear history
the workflow with three MRs would look like:
- Reviewers have finally approved!
- You enable "Merge when pipeline succeeds" on all three MRs
- You are done as all three MRs will be automatically merged once CI has finished
The whole rebase business together with its bugs are gone.
The idea of "Rebase without pipeline" does IMHO not make sense at all as the tests are not run and that was the whole point of rebasing or?
Wit the proposed change we do have a slight potential risk, that the the suggested changes from one MR would get broken by another MR which got merged earlier.
But I think we can live with that risk due to:
- It happens rarely
- We can recover quickly
- We do have rc cycles before releases and not a rolling release
Decruft the MR list
Problem:
Difficult to find an MR which is worth doing code review on.
Solution:
Close all MRs which target something different than the next release. After closing the MR we should rename the branch and make a note in the issue so that we can find it again.
So %Some day MRs can be closed and in case 9.4.2 is our next release, we also close %9.5 and decide what to do with %9.4.