Multi-line merge request comments MVC
Problem to solve
Commenting on a single line is great for simple kinds of code review feedback, but frequently a comment is actually addressing multiple lines in the diff, perhaps a portion of a logic block, a paragraph of prose or an entire function. This forces users to chose a single line to provide feedback, but the feedback may in fact be resolved with a change to a different line.
Multi-line comments presents a range of challenges that make it difficult to implement in a single iteration. Particularly the complex and substantial changes to the frontend are the primary area of concern.
It should be possible to create a comment on a merge request diff that spans multiple lines. It should be viewable by others, resolvable and possible to reply to, including:
- internal APIs to provide this functionality to the interface
- public APIs (including) may not be updated in this iteration, as multi line comments will can be treated as single line comments targeting the last line in the range (where they will be displayed in the interface)
The primary area of scope reduction for the MVC is in the frontend.
As a reviewer, I will be able to create a multi-line comment by:
- clicking on the last line in multi-line range I wish to comment on
- expanding the range from one line, to many lines through a very basic interface
Additionally when viewing a multi-line comment:
- there should a visual method for seeing the range to which the comment applies
- this may be a hover state,
- or for simplicity a text description of the range in the discussion interface
As discussed in the solution validation issue, for the PoC, given the complexity of the implementation of the desired visual representation of overlapping comments, let's keep it simple:
- Basic UI to increase the range of lines when leaving a comment. (example, not final mockup)
- Basic display on hover and when the comment is focused/clicked.
Availability & Testing
- No risk to availability is expected, but we should check how the feature performs on large diffs with many comments.
- Given the complexity of the existing functionality this feature builds on, substantial new unit and integration coverage will be needed to cover known troublesome edge cases. Particularly interactions with expanding lines. Feature tests may be justified for additional confidence.
- We should also confirm there are no unintended changes to the Web IDE.
- No need for a new E2E test, and the change isn't expected to affect existing E2E tests.