Batch Comments: deal with pending comments on lines that get changed before publishing review
Follow-up to discussion in https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/7376#note_106107152
Scenario:
- Reviewer writes a pending comment on a code line of a merge request
- A few moments/minutes/days pass
- A commit is pushed that changes that line
- Discussions/published comments on that line will become outdated and not shown
- Pending comments, however, will stay on the line until published and will result in a comment on the latest version of the merge request at the time of publishing.
Pending comments stay because we felt it was the author's responsibility to decide whether it still applied or not, upon publishing.
Proposal:
Add a warning to pending comments saying the content of the line has changed in a recent commit OR that the changes to the file have been reverted in a recent commit — so the comment will not be published on the most recent commit but instead on a previous commit.
Considerations:
- Pending comments can become “irrelevant” when referencing an old line of code or a file that is no longer part of the merge request (because all changes to a file have been reverted in a recent commit).
- We should warn users not only after saving a pending comment, but also during writing in real-time, because a commit might come in the meantime that makes that pending comment “irrelevant”.
Reproducing
- Leave a review note in a diff file in a MR
- Update a few lines above the commented line with two or more breaklines
- You'll note that the review note vanished, but you still can send the review
- You'll note that the position of the review note was not updated (hover the Submit review button)
Edited by Oswaldo Ferreira