Show code quality notices inline on diffs/MRs
Release Notes
Developers authoring or reviewing a merge request want to know the impact that change is making to the quality of a project. To do this within GitLab they have to have two windows open, one on the code and one on the Merge Request page to see the Code Quality changes. This back and forth is inefficient and makes it easy to miss critical code quality issues.
Now you can see in the Merge Request changes tab the line that introduced a code quality violation and severity so you can quickly identify the most critical issues to resolve as part of that Merge Request. Hovering over the icon provides details about the violation.
Problem to Solve
When considering changes between branches/in an MR, one of the most important factors for teams to consider is impact to quality. We already offer quality metrics, but we can improve this by showing code quality remarks in the diff prior to accepting a change.
Intended users
- Sasha (Software Developer) - who is reviewing a change from a team member and wants to see the code quality issues from the Merge Request widget in the diff view for easier context.
User experience goal
The user should be able to see code quality violations in context of the source in the MR diff view of the GitLab UI.
Further details
This utilizes the existing data from the CodeQuality scan.
Acceptance Criteria
When changes to code quality exist, the following acceptance criteria is valid:
- Add an icon indicating code quality severity to the diff area of a merge request.
- Icon size should be
12x12
pixels. - We will display a different icon for each severity level. UI of icons should replicate the ones defined in #199187 (closed)
- Icon size should be
- Only one (1) code quality severity will be display per line.
- When a code quality severity icon is present, the indentation of the lines in the diff after the number of the line needs to be increased by
20px
. The code quality icon is added before the + or - sign in the diff UI. - Hovering the icon should display a tooltip that reads the cause for the severity alert.
- Clicking the icon should trigger a modal. The modal should display:
- Title: Code quality
[degraded/improved]
:[description of the degradation]
- Body: description, file path and line number, severity.
- Footer: close button.
- Title: Code quality
Out of scope for this iteration
- It happen that multiple code quality changes happen to the same line of code, but this MVC ignores that case for now and just shows the first code quality degradation.
- The severity icon does not support a text label for now. Only a tooltip is displayed on hover.
- Updates to the color or the severity icons are also out of scope, and this MVC should replicate the existing icons/color available in the application.
- No other actions (such a giving the user the ability to fix the vulnerability, or open an issue) will be available in the modal.
The Design for this can be seen below and in the Figma prototype.
Proposal
When a code quality report exists on a merge request the code quality violations can be shown for the existing and added code. Display them as shown in the design below.
Permissions and Security
Documentation
- Update the feature documentation with content about what uploading a code quality report on target and source branch does in MRs.
Availability & Testing
The biggest risk for this is load time on the diff view. Ideally this will be behind a feature flag and can be enabled on a project by project basis for early testing/dogfooding within gitlab and Self Managed customers as they desire.
What does success look like, and how can we measure that?
Acceptance Criteria
-
When a Merge Request Pipeline has a Code Quality job that runs successfully the Changes tab will show annotations as noted in the design in the code. -
Time to first paint for the diff view increases by less than 5% as measured on the gitlab-org project before fully enabled. -
Text has been approved by Tech writer (@eread for this issue) -
Documentation for the feature is updated to show this capability / how to enable it (upload files) -
Tracking is put into place for users at the proper license tier who view the mr diff view AND have code quality data loaded
Measures of Success
- Within 90 days of launch we expect 5000 unique users per month to view a diff page with code quality content on it.
- Tracking will be done in #267626
What is the type of buyer?
Alex and Dakota want their team(s) to start tackling the tech debt identified by GitLab's Code Quality scanner and know that it's too hard to do this as part of Code Reviews today.
Is this a cross-stage feature?
Yes, the groupsource code team has been notified.
Links / references
These designs were used for the new designs in the design tab below. BE prepares the new API to present the annotation to FE. The internal JSON data would be FE overlay the results on diff view in merge requests. Initial state: Hover state: Existing comment on line state: Side-by-side diff example: Modal: Toggle to hide inline all code review remarks:
Frontend
{ file_name: "xxx", "line": "xxx", "description": "xxx" }