Comment on merge-ref diffs
Problem to solve
Follow up to #27008 (closed)
Merge request diffs are currently calculated by git diff target...source
which compares HEAD
of target
with the merge base of target
and source
. This works well until changes from the target branch are merged in to the source branch, creating a complete mess of the diff.
In #27008 (closed), we implemented a new diff mode to fix this, but it doesn't support commenting, and is not the default mode because of this.
Proposal
- Add the ability to view and add comments to the merge-ref diff
- Add a feature flag so that we can make the merge-ref diff the default diff mode for specific projects
Since the default diff mode is reliable in most instances, very few people will try a new mode. So that we can ultimately make the merge-ref mode the default and remove the current default mode, we need to test it at scale. Using a feature flag will allow us to access performance, scalability, and reliability of this feature.
Availability & Testing
Risk to availability should be sufficiently mitigated by gradually rolling out the change behind a feature flag, and by users having the option to return to the original diff mode.
Given past and present problems with comments disappearing from the discussion history (e.g., after a rebase), we should confirm that we have suitable test coverage of typical and edge cases, and that the coverage if valid (or added) for the new diff mode, including that:
- comments are still shown on the diff after changes (commit/merge/rebase/squash/revert) to the
target
orsource
(assuming the line with the comment wasn't removed) - comments are always accessible on the Discussion/Overview tab (even after changes that hide them on the diff)
We only have limited end-to-end test coverage of diff comments, so this would be a good opportunity to add more. However, unit/integration test coverage should be sufficient, so this work shouldn't wait for an end-to-end test to be ready.
-
package-and-qa
should be run before merging to check if existing end-to-end tests need to be updated.