Merge Request Reviews

Maintained by:

Overview

This page outlines the guidelines for Product Designers when reviewing merge requests (MRs), also called “UX reviews” or “Product Design MR reviews”.

Requirement

Almost all merge requests (MRs) change the UX in one way or another, but Product Designers are only required to review and approve MRs that include user-facing changes. Per the approval guidelines, “user-facing changes include both visual changes (regardless of how minor) and changes to the rendered DOM that impact how a screen reader may announce the content.”

MRs with only backend changes sometimes affect the UX (for example, performance changes, sorting of lists, etc.), but you are not required to review them unless they fall under the definition of “user-facing changes”.

To help triage, be aware of all MRs in your stage group and ask engineers about which MRs could affect the UX and how. Product Designers often give constructive feedback on any kind of MR, including MRs that seem to not affect the UX, so use your best judgment when deciding which MRs you should review.

How to assign MR reviews

Stage group MRs

The GitLab Roulette will randomly suggest a designer to review. These MRs should have a well-documented design solution. Your role as a reviewer is to ensure the solution is developed as intended. After you've completed your review, cc the design DRI of the group for awareness. Additionally, if you have further feedback on a particular UX solution, open a follow-up issue and mention the design DRI.

There may be scenarios where an MR is based on a previous conversation or it proposes a solution that did not have a design associated with it. If this happens, @ mention the design DRI to ensure requirements are met. The design DRI should then notify the suggested UX reviewer to complete the MR review when ready.
- An example is an MR with the label, ~"Internal hackathon". The design solution is likely being solved at the time of development.

Community contributions

Community submitted MRs require more context to review and are assigned to the design DRI of the group it impacts. All UX-related community contributions will generate a Slack message in the #ux-community-contributions channel and will be manually assigned until we automate the process.

Single engineering group MRs

Single Engineer Group (SEG) MRs often require more active collaboration and additional context on the work. UX-related MRs created from a SEG should be reviewed by the design DRI of the group it impacts.

UX Paper Cuts MRs

MRs created by the UX Paper Cuts team will be handled by the UX Paper Cuts team. If one of the two designers are OOO, then the team will utilize the GitLab Roulette to help distribute MR reviews across the Product Design subdepartment.

Benefits

Evenly distribute MR reviews across the Product Designers in the UX Department in support of our Product Design MR review volume KPI. This will have a more significant impact on higher usage areas of our product by opening up capacity for those designers to increase time spent on creating design solutions.
Create awareness of the end-to-end GitLab product by having designers involved in reviewing work outside of their normal areas. This can lead to holistic solutions and seeing connections between stage groups (and Pajamas) that would otherwise get missed.
Improve communication and documentation of design solutions, as we're now relying on another designer to confirm that a solution is developed as intended.
Give designers visibility and experience in other areas of the product to potentially discover a new passion or area of interest.

Workload and response times

MR review requests are the number one priority for Product Designers. Respond to them per our review-response Service-Level Objective.

Balancing MR reviews with other priorities is a challenge, as they can be unpredictable. To avoid disruptions and context-switching, we suggest you block some time every day to review MRs (for example, 30 minutes or 1 hour per day).

If you're struggling with MR reviews, remember to manage expectations with MR authors and let your manager know right away so they can help you. Before taking up a review, make an estimation of your capacity keeping in mind any upcoming time off. If you are unsure of completing the review before leaving for a time off, spin the roulette again to assign the review to a different designer.

Reviewing

In general, follow the Code Review guidelines (it's a long document, but please read all of it). Exceptions to those guidelines are noted below.

Understand the MR

Check the MR description for the following information. If any are missing, ask the author to complete the description before investing time in the review.
- A thorough explanation of the changes.
- How you can test the changes.
- Link(s) to the related issue(s).
- Before and After screenshots/videos (if appropriate).
To reduce waiting time for previewing the MR in a live environment (like Gitpod, your local GDK, or even a Review App), start the environment and let it load in the background while you familiarize yourself with the MR and related issue(s).
Some MRs with the ~"UX" label may not have a design DRI or proposed designs associated with the change. In these cases, it is important to get as much context as you can about the change. If you are unsure how the changes impact other areas of the product, consider bringing in other designers and your design manager to help evaluate the impact of the proposal.

Preview the MR

Always review the MR in a live environment, so that you can experience the changes as our users will and review them thoroughly. To choose the most appropriate method to preview the MR and get started, see Review, test, and contribute. If you get stuck, don't struggle on your own, check our help section.

Specific review requirements

Some MRs have additional set up requirements.

SaaS-only features: For testing SaaS-only features you'll need to run the GDK as a SaaS version. Instructions for simulating SaaS version in the GDK
Paid features: You'll need a license for these. Request a license via and Access Request, using the GitLab_Team_Member_License_Request template. Instructions for adding the license to your instance.
You can also switch between CE and EE editions: How to simulate a CE instance
Pipeline-related features: You'll need to enable a runner to run a pipeline. Review apps have runners available by default. Instructions for enabling a runner in Gitpod or GDK.
Compliance: To test audit event streaming MRs you might need to set a stream destination URL. You can use Pipedream to generate a temporary destination.
Fulfillment: Because of the complicated setup required for CustomersDot, only Fulfillment Product Designers should review CustomersDot MRs and GitLab MRs that require connection to CustomersDot (e.g., the GitLab.com purchase flow).
- Instructions for setting up CustomersDot locally.
- There are times when it isn't possible or practical for a Product Designer to complete their review via their local environment. At these times, the Product Designer can review screenshots and videos in the MR description or coordinate a demo with the Engineer. Another option for more complicated changes is keeping the change behind a feature flag and reviewing the change on staging after the MR has been merged. This is up to each Product Designer's discretion.
Geo: Using the simple installation script, you need to install and configure two GDKs. The script does not work on GitPod. The configured GDKs served as a Geo primary and secondary site.
- Instructions for the simple installation.
- Video instruction and slide deck for setting up Geo-GDKs

Pipeline Execution: For CI/CD minutes and shared runner usage related features, you need to populate projects with historical CI/CD minutes usage data to test the features or changes.

Merge requests related to CI/CD minutes and shared runner usage usually require some historical usage data, which can be difficult to set up if it doesn't exist already on the local GDK environment. Below is a video and instructions for how to set that up in under 7 minutes.

Checkout the branch in the MR and open rails console using bin/rails console. 1. Edit CI minutes

ApplicationSetting.current.update(shared_runners_minutes: 400)
project = Project.find(20)
root_namespace = project.root_namespace
namespace_usage = Ci::Minutes::NamespaceMonthlyUsage.find_or_create_current(namespace_id: root_namespace.id)
Ci::Minutes::NamespaceMonthlyUsage.update_counters(namespace_usage, amount_used: 100, shared_runners_duration: 100)
project_usage = Ci::Minutes::ProjectMonthlyUsage.find_or_create_current(project_id: project)
Ci::Minutes::ProjectMonthlyUsage.update_counters(project_usage, amount_used: 100, shared_runners_duration: 100)

Type :wq to exit the log lines. Do not exit the rails console.

2. Add helper method to rails console

def increase_ci_usage(project:, date:, amount_used:, shared_runners_duration:)
date = date.utc.beginning_of_month
project_usage = Ci::Minutes::ProjectMonthlyUsage.where(date: date).safe_find_or_create_by(project_id: project.id)
Ci::Minutes::ProjectMonthlyUsage.update_counters(project_usage, amount_used: amount_used, shared_runners_duration: shared_runners_duration)
root_namespace = project.root_namespace
namespace_usage = Ci::Minutes::NamespaceMonthlyUsage.where(date: date).safe_find_or_create_by(namespace_id: root_namespace.id)
Ci::Minutes::NamespaceMonthlyUsage.update_counters(namespace_usage, amount_used: amount_used, shared_runners_duration: shared_runners_duration)
end

3. Use helper method

increase_ci_usage(project: project, date: 1.month.ago, amount_used: 10, shared_runners_duration: 20)

The usage quota page should now reflect the changes data.

Secure:
- To generate project vulnerabilities, execute GITLAB_QA_ACCESS_TOKEN=XXXXXXXXXX GITLAB_URL="https://gitlab.com" bundle exec rake vulnerabilities:setup\[<Project_Id>,<Vulnerability_Count>\] --trace from the gitlab/qa directory. Make sure to replace the placeholders in the script with your local access token, project ID, and desired number of vulnerabilities. An example of this might be GITLAB_QA_ACCESS_TOKEN=asdfASDF1234- GITLAB_URL="http://localhost:3000/" bundle exec rake vulnerabilities:setup\[25,10] --trace
- To populate a merge request with vulnerabilities, follow these steps.
Monitor:
- For some MRs, you may need to add test alerts. Instructions for adding an alert, along with a sample alert, are available here (see the inline code snippet of instructions).
- Service Desk MRs sometimes require setting up incoming_email, service_desk_email and MailRoom. These MRs can't be reviewed on GitPod and need a working GDK. Follow the set up instructions in the GDK docs or this video walkthrough.
Value Stream Analytics: Value Stream set up and seed data instructions. Many VSA features require an EE license, request a developer license.

If you find yourself struggling to get your environment set up, try reaching out to the design DRI for help with the specific testing requirements. If you're still having difficulties after an hour or so, it's perfectly acceptable to hand the MR over to the design DRI for review completion. If you have time it might be worthwhile to shadow the design DRI to watch and learn how they complete the MR review.

Review the MR

Review using the design and UI changes checklist to make sure all main aspects are covered.
- The team can decide to merge before a full review if the changes remain behind a feature flag and there's a plan for you to do a full review in staging. Beware that this approach can result in unplanned issues and MRs if things need to be fixed.
Stick to the UX requirements in the issue. See the follow-ups checklist on creating issues for further updates or missing pieces.
Follow our review best practices for everyone and reviewers.
- Remember that reviewing someone else's work is a dialogue and is an opportunity to build trust and rapport within your team.
Separate each topic into its comment thread so that they can be discussed and resolved separately. If possible, create threads on the line(s) of code related to the topic.
Ensure the author is clear on what is required from them to address/resolve the suggestion.
- Consider using the Conventional Comment format to convey your intent.
- For non-mandatory suggestions, decorate with (non-blocking) so the author knows they can optionally resolve within the merge request or follow-up at a later stage.
- There’s a Chrome/Firefox add-on which you can use to apply Conventional Comment prefixes.
Share annotated screenshots or screen recordings in your comments whenever possible. Visual materials make issues clearer and communication more efficient.
- To capture your screen, use free apps like CloudApp, Monosnap, or Mac's Screenshot (see how to capture and annotate).
- Annotate screenshots to highlight specific aspects that need to be addressed.
- Highlight differences between what's implemented in the MR and what's expected with a Markdown table that has images/videos. Consider using the template below.
  Differences table template
```
| This MR     | Expected    |
|-------------|-------------|
| Image/video | Image/video |
```
Try to find something worth praising the author for, like a thorough MR description or their attention to detail on a certain aspect. But don't make empty praises, only praise them if you recognize the value of what they've done.
When there are concerns with an MR, especially for those that did not have a design DRI associated with the proposal, consider the following actions to help progress the review:
- Iterate instead of revert. Consider coaching and guiding the author of the MR towards an ideal solution.
- Educate to collaborate. You may have more context of the problem than the author. Express your concerns and thoughts, and ask them how might they adapt their solution to accomodate the concerns.
- Get a second opinion. When you are uncertain about a change feel free to involve a quad member from your product group, your product design manager, and/or other designers. This can be done directly in the MR or in Slack channels.
- Consider creating a follow-up issue to address the concerns of the MR and inform the design DRI of it.
- If changes are behind a feature flag, but there are concerns about the feature going live, consider creating an issue that blocks the full release of the feature with a list of items that need to be addressed (see this example).

Handoff the MR

After each review round, you should remove yourself as a reviewer and post a summary comment, letting the author know if changes are required following your review.
To address any outstanding UX concerns that deviate from the MVC, you should create follow-up issues and label them as UX debt (learn more about this label in how we use labels).
If you complete an MR for work that isn't within your specific group, you should consider following up with the original author to discuss their documentation (ex. "This was unclear… In the future, you may want to…"). This can be thought of as a casual retrospective and can be synchronous or asynchronous.
When you're confident that the MR meets all requirements you must approve it by hitting the Approved button in the merge request widget. To handoff the MR, see the responsibility of the reviewer.

Performance indicator

The Product Design merge request (MR) review volume is tracked as a Key Performance Indicator (KPI) of the UX department.