Provide convenient way to view Merge Request Incremental Diffs between source versions when a rebase is involved
Note that on MR updates, when no source rebase is involved, the Compare with previous version
works great, and helps identify the exact changes since the previous version. However, the experience is not very kind when rebase is involved.
Proposal
In our monorepo, the target branch (master
) moves very fast, and brings in a lot of commits and changes from various teams that share the monorepo. As a developer, we also have some workflows that encourage a rebase of the source branch against the target prior to both creating and updating Merge Requests. Upon rebases during updates, the source version is now based off of 100s of new monorepo commits that landed on the target since the previous push to the MR.
GitLab uses the diff algorithm described here. Accordingly, the source is merged into target creating a synthetic commit, which is then ..
-diffed against the target, which shows the correct changes in source that are not in target. This is perfectly fine for master (HEAD)
vs latest version
. However, if I'm interested in incremental diff between any two source versions, your algorithm doesn't work that great.
Let's consider this source setup:
F - G H - I
/ /
A - B - C - D - E
where the A-B-..-E
chain is the target branch progress. The MR was originally created/updated based off of B
with source branch pointing to G
. Let's say that the reviewer requested some changes on the MR. Separately, over time, commits C-D-E
have landed on the target branch. Let's say that the author incorporates the changes and squashes them into G
to create G'
. Next, the author chooses to rebase against target (E
) and push to the MR. The source branch is now pointing to I
which is basically the rebased G'
commit.
Now, as a reviewer, in GitLab UI, if I try to view the incremental diff (by selecting Compare with previous version
in Overview tab), I want to see the exact changes the author did to go from G
to G'
including any incidental changes that were fixed up on rebase to I
. However, in GitLab UI today, your algorithm will pick up and show the necessary changes, and ALSO the source changes from commits C-D-E
which is not relevant to the author's changes at all. This makes the reviewer workflow unnecessarily complicated.
Now, scaling that example to our monorepo, typically, on rebase, we pick up 100s of new commits that land on the target branch. Even in a normal case, it gets so bad that GitLab UI even fails to render the diff because the changes are so big between versions, mostly because of commits and changes that are not even relevant to the author's changes (the C-D-E...
above). Our users end up seeing:
even on rebased one-line changes. This is unacceptable, and has definitely degraded our code review experience/workflows. To be fair, even the traditional target...source
diff algorithm doesn't work too well with incremental diffs.
Anyway, the only way we are able to proceed with pushing our users to use GitLab for code reviews, is by:
- Asking authors not to rebase their changes going forward, since it breaks the reviewer workflow (particularly, reviewing incremental diffs). If they must rebase to fix a merge conflict, we ask them to rebase and fix the conflict, but we ask the reviewer to accept that source version blindly since they simply cannot review just the changes from the conflict-fix due to the rebase. Neither the reviewer, nor the author is happy with this.
- Assuming there are rebases on push, we ask the reviewers to rely on
master (HEAD)
vslatest version
to review every time. Our reviewers are not happy because on every single push, they must review every single file even though there are no changes. Even if you change a single character in a file, you are compelled to visit every single change on that file since theHEAD
version.
Our users have accepted the above workarounds very reluctantly, which has allowed us to start pushing more users to use GitLab MR for code reviews. However, they have definitely pushed back in numbers, and asked for GitLab to consider supporting this feature natively.
I'm not a git expert, and cannot suggest a perfect implementation of the diff algorithm that will help elide the changes from the rebase that is not relevant to the source or merge request. However, newer versions of git has git range-diff, which I believe can get us what we are asking for. In the above example, doing a git range-diff B..G E..I
should yield the right diff. Note that the diff output is differently formatted than git-diff
, and might require minor data rendering/mapping changes as well. But in my opinion, this should be diffable. If it is diffable, given the win from that, it seems reasonable to provide a means to view this diff so as to not concern oneself with unrelated changes from rebase.
We would like GitLab to implement this feature whereby we can elide unwanted changes/commits from a rebase when performing incremental diffs in Merge Requests.
Note that there is an open issue raised 2 years ago that is similar to this request. See #229554
Let me know if you need anything else from me or Two Sigma here. Thanks!