Skip to content
GitLab
Next
Closed
Issue created by Annabel Dunstone Gray@annabeldunstoneMaintainer

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 and Comment; how can we convey "disapprove" in a kind and actionable way?

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

The next design should tackle:
  • 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)
  • 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
Edited by Annabel Dunstone Gray