Review rounds: Initial design exploration
Issue
Following up from https://gitlab.com/gitlab-org/ux-research/-/issues/1769+, one of the main concerns participants had was the fact that the comment would be posted as a standalone comment, rather than as a threaded response to the original review request comment.
Proposal
Following implementation of 🎨 Design: Add comment and approval as part of c... (#350394 - closed) Brainstorm how we can make the entire review more of a structured, end-to-end process.
Braindump
- User selects reviewer and has ability to add a comment
- System note and review request comment are posted together as a single object
- Reviewer submits review
- Code comments, summary comment, and system note are posted together on that original object/thread (maybe code comments are collapsed
🤔 ) - Add all pending comments to overview tab (with links to the actual comments on whichever commit) to give complete picture of review
- Add additional options when submitting review
- We are already adding
Approve
andComment
; how can we convey "disapprove" in a kind and actionable way?
- We are already adding
Version 1: basic designs of one potential flow
Author: Click request review Assign reviewers for each role (frontend, backend, UX) 3 reviewers are assigned 3 new activity entries are added Each reviewer gets MR added to “Review requests for you”
Reviewer 1:
Banner at top of MR (@author
has asked you to review the frontend (link to activity entry)
Reviewer can click Start review
from banner, activity entry, or comment (current way)
When review is completed, it is added to activity entry (can collapse entire review)
Review can either approve, request changes, submit review without either
Approved ->
- check mark appears next to reviewer in sidebar
- MR removed from “Review requests for you”
- Todo/notification generated for author
Request changes ->
- Banner appears for author saying
@user
requested changes (link to activity entry) - Icon appears next to reviewer in sidebar
- Can re-request review in activity entry or sidebar
- MR removed from “Review requests for you”
- Todo/notification generated for author
Just review (no approval or required changes; maybe just comments or questions)
- Banner appears for author saying
@user
requested changes (link to activity entry) - Icon next to reviewer in sidebar
- Can re-request review in activity entry or sidebar
- MR removed from “Review requests for you”
- Todo/notification generated for author
Version 2: Utilize code owners and approval rules, add MR statuses, filters, and batch reviewer selection
Version 3: Top nav counter and MR list
Version 4: "My merge requests" list, conventional comments, de-duplicating codeowner approvers, quick actions, non-required approvals
- Status piece is tricky and needs more re-work
- Incorporate quick actions
- Conventional comments / defect classification (blocking vs non-blocking) — could function as “request changes”
- How to display rule that triggered reviewer type?
- Re-request review after approval given
- Add global actions approve/review/submit review
- How can we incorporate this in a more flexible way (show options even when code owners aren’t required, make sure it works for the way gitlab works)
- Make sure “MRs that require my attention” aren’t too alert-y
- Check flow for smaller MRs (for example when a team only has all frontend MRs or all backend MRs)
- When users meet criteria for multiple codeowner/approval rules
- Separate "MRs I've authored" from "MRs I've been requested to review" from the "attention requested" section of the MR page
I'm going to remove the following items entirely:
- Filtering (based on discipline, review type, activity type, etc)
- This is something we're exploring more in https://gitlab.com/gitlab-org/gitlab/-/issues/365334+
- Notifications and todos
- These have their own problems and I don't want to go down a rabbit hole trying to solve them with review rounds (more on notifications work here)
- For now, we'll likely go with default behavior- maybe you'll receive a todo/notification based if you've been requested to review an MR or if a review has been submitted on your MR. More on that later