Discussion: Generalize Merge Request Items
Merge requests currently have essentially two core interactive items:
- Diff Lines (which are part of Diff Files)
- Discussions/Notes
The design of the front end application has grown organically from the ad hoc needs of building Code Review.
In the spirit of wild ideas, we should consider reassessing how Merge Requests are designed, at least from a technical perspective.
Problems
Here is a non-exhaustive list of problems we have encountered or may encounter in the future with the current paradigm:
- Lines of code are not the most basic building block of a diff; commenting on a single variable name or other parts of a code AST is a reasonable - and unmet - need.
- Content outside of the actual changes (e.g. a related-but-unchanged part of the code) is not conceptually part of a "diff" - and therefore not conceptually part of a merge request.
- The MR metadata (title, description, etc.) are not interactive in the same way code/binary files are.
- Notes on the MR are technically different from Discussions on the Diff.
Example Workflows
To demonstrate features we could introduce that would be difficult or impossible in the current application, here are some future workflows:
- A user notices that an unchanged part of the diff already has features that are being implemented in the changes in the MR.
The user leaves a comment on the unchanged part of the diff, telling the author that what they're trying to do is already present.
Problem: The author cannot see that comment in the Changes tab, because the comment is outside the understood scope of the diff. - A user wants to comment on a single parameter name in a function.
It is highlighted as a unique part of the syntax tree, and they're not happy with the particular name the author of the MR chose.
Problem: The user can only comment on the entire line, because "lines" are considered the most fundamental building block in our Diffs UI. - A reviewer would like to open a discussion on the MR description because they disagree with some of the conclusions, but want more clarification.
Problem: The only way to have a conversation about the MR metadata (like title, description, labels, etc.) is to open a generic note on the whole MR. There's no way to scope a discussion to a part of the MR. - A reviewer wants to view all comments on an MR in a condensed list, and - for each one - see the context of that comment.
Problem: Notes (on the MR) and Discussions (on the Diff) are treated differently, and Discussions necessarily come with Diff context. Diff context with Discussions is difficult to match with actual diffs because rendering diffs isn't designed for non-Changes-tab contexts
Proposal
To begin rethinking MRs in a way that can accommodate many of these concerns, we should break down and MR into "Blocks" (trademark pending):
- Code block
- Metadata block
- Note block
- MR block (we should still be able to interact with the MR as a whole thing, e.g. leaving a generic comment on the whole thing)
These blocks could be further specialized with more specific behavior:
- Code: Node, Line, File
- Metadata: Title, Description, Label, File (e.g. represents the title, lines changed, mode changes, statuses like collapsed or viewed)
Each block can be optionally Noteable. For example, it's unlikely that we want File Metadata to allow comments.
Note blocks are then associated with some other kind of block (e.g. it's a note on an MR block or it's a note on a Code Node block), and have an appropriate renderer.
Theoretical MR Structure based on Blocks
Code Nodes can be combined (essentially via AST) into Code Lines, which can be combined into Code Files, which are children of an MR Block.
MR Blocks also have Title, Description, and Label children. Code Files have Metadata File blocks in addition to their Code Line children.
Theoretical Outcome
- By generalizing the individual items of an MR and dissociating (for example) Notes from having specific parent types, we can expand the ways that an MR can be put together. For example, by recombining all of our Note Blocks into a single view, we can offer a "Conversations" view.
- By abstracting code away from "Files" and "Lines" we can offer more nuanced views like "Repeated Code Blocks" that could be refactored.
- By only connecting blocks to other blocks, we can standardize the mechanics of how (or if) those relationships display. Importantly, we can also standardize how we store and retrieve those relationships.