Skip to content

Create POC for MR review feature

Problem to solve

Before we implement #53 (closed), we'd like to validate certain assumptions like:

  • GitLab API provides us with the MR diff file content and comments in an easy-to-consume format
  • VS Code Extension API allows for easy integration of the GitLab API results with the diff view

Proposal

Implement POC that can:

  1. Show files that have been changed in the MR
  2. For each one of those files shows a diff view between merge-base and the MR branch
  3. Display comments from the MR diff
  4. Allow creating comments on the diff

Report from POC implementation

This report is a summary of the POC work. It will try to explain what uncertainties we've removed and discovered.

Extension_Development_Host_-_test_js__MR____test-project_and_mr_item_js___gitlab-vscode-extension_and_zsh

Try it yourself

Straight to the fun bit, try it yourself by installing the packaged extension with POC (how to install)

Caveat: All MR refs (commits) need to be available locally (run git pull before trying the POC)

I've tested the POC on gitlab-org/gitlab project and it seemed to work for most of the MR's.

The messy code is available in mr-review-poc branch.

Features:

File tree:

Extension_Development_Host_-_index_md___gitlab

Certainty overview

  • Dependencies available and working: 4/5
    • REST API ✅, GraphQL ❌
  • VS Code API suits our needs: 4.5/5
    • Only the Git Changed status indicator is unavailable to us.
  • The codebase is ready for implementation: 3/5
    • Tree view cleanup necessary before starting

Details

There is no custom UI involved; we are going to utilise the VS Code API.

We can get necessary information about the changed files from the REST API (MR version). The GraphQL API can't provide us with the necessary information (gitlab#280803).

Creating the file tree seems straight forward, the only thing I wasn't able to achieve vas to show the Git Changed status indicator:

Extension_Development_Host_-_snippet_blob_actions_edit_vue___gitlab

The API seems not to be officially exposed.

When talking about the design, we'll have to decide between two ways of showing the changes:

Tree Path in the item detail
constants_ts___gitlab-vscode-extension constants_ts___gitlab-vscode-extension

Known issues and uncertainty

  • The current code (issuable.js and sidebar_tree_item.js will need a cleanup before we can start build on topo of it.
  • We'll probably always work with the last MR version and will ignore the intermediate versions.
  • We might want to start showing Merge Requests differently from issues in the Tree View. Also we might want to choose icons for all tree items.

File Diff View

Extension_Development_Host_-_terraform_list_vue__MR____gitlab_and_Add__Ability_to_chec______gitlab-vscode-extension_and_zsh

Certainty overview

  • Dependencies available and working: 3/5
    • fetching remote content has not been tested
  • VS Code API suits our needs: 5/5
  • The codebase is ready for implementation: 2/5
    • It would be best to slowly shift Git related responsibility to VS Code (instead of running shell command)

Details

VS Code provides us with the diff functionality, the only thing we do is defining a custom document URI and then implement content providers for that URI.

Uri:

"gl-review://authority/app/assets/javascripts/terraform/graphql/queries/get_states.query.graphql?commit%3D5fa9332d0cedb98b0095b34d01470e419d3a98ba%26workspace%3D%2FUsers%2Ftomas%2Fworkspace%2Fgitlab%2Fgitlab-development-kit%2Fgitlab%26version%3Dbase"

When VS Code tries to open our URI, it will invoke our content provider, which returns a string content of the file. The POC implements only the local git content provider, which means that the file version (correct commit) needs to be in the local git repository. The proper implementation is going to implement a fallback provider which will fetch the content from the API.

graph LR
A[VS Code] -- "gl-review: URI" --> B[LocalGitContentProvider]
B --> C[RemoteApiContentProvider]

The future-proof way to interact with git is to use the VS Code core extension for Git (originally suggested in !54 (closed)). It is how most of git-related extensions work. The added benefit is that everything git related (VS Code VCS view, other extensions, our extension) is going to be in sync. This will require a bit of plumbing implementation, potentially overlapping with Consistent handling of remotes, instances and branches.

Known issues and uncertainty

  • I haven't implemented the API file content provider and so we don't have 100% certainty that we can retrieve the content.
  • I'm not sure how are we going to react to remote changes. For example, when the user refreshes the file tree and all of a sudden the displayed diff doesn't correspond with the latest version.
  • I'm not sure if there is a difference in how we'll use API to fetch file content from forked projects
  • Images - VS Code can show images in tabs, but the POC haven't done any exploration of how that works.

Showing existing comments

Extension_Development_Host_-_test_js__MR____test-project

Certainty overview

  • Dependencies available and working: 5/5
  • VS Code API suits our needs: 4/5
    • GitLab-specific highlighting like #243 (closed), !54 (closed) and @viktomas is not going to work
    • images and advanced Markdown syntax (mermaidjs) is not working out of the box and maybe not at all
  • The codebase is ready for implementation: 5/5
    • This is going to be a greenfield development.

Details

This feature is going to use GraphQL, so far it seems to be the only endpoint that's going to work out of the box.

See the full GrapQL query 
 query GetDiscussions($projectPath: ID!, $iid: String!) {
    project(fullPath: $projectPath) {
      mergeRequest(iid: $iid) {
        discussions {
          nodes {
            notes {
              nodes {
                body
                author {
                  name
                  avatarUrl
                }
                position {
                  diffRefs {
                    baseSha
                    headSha
                  }
                  filePath
                  positionType
                  newLine
                  oldLine
                  newPath
                  oldPath
                }
              }
            }
          }
        }
      }
    }
  }

The VS Code API allows us to add comments for our custom URI. This seems to work really well (and potentially handles automatically comments on outdated diffs). Each comment has a body (markdown string) and Author information. We create the commits as follows:

commentController.createCommentThread(
  getReviewUri({
    path,
    commit,
    workspace: this.projectUri,
    version: old ? 'base' : 'head',
  }),
  new vscode.Range(vsPosition, vsPosition),
  comments,
);

Known issues and uncertainty

  • I assumed the following about the GrapQL response
    • Each GitLab comment has a diffRefs and I assume they always point to the latest MR version (even if the comment was made on the previous version).
  • The POC didn't validate editing or deleting existing comments.
  • Reactions to the comments are possible but weren't part of the POC.
  • As mentioned in the "Certainty overview", some markdown might be not rendering correctly
  • The comments can range over multiple files (according to the API) but it hasn't been tested

⚠️Creating new review comments⚠️

pullRequestNode_ts__Pull_Request____vscode-pull-request-github

Certainty overview

  • Dependencies available and working: 1/5
    • I wasn't able to reliably create a comment (note) on GitLab MR through our API.
  • VS Code API suits our needs: 3/5
    • It should because other extensions work, but parts of the API are not officially documented.
  • The codebase is ready for implementation: 5/5
    • This is going to be a greenfield development.

Details

First, we tell the VS Code where in the file can we create comments. VS Code provides an API for that: CommentingRangeProvider. This provider gets a URI and returns ranges of lines where it is possible to comment.

My understanding is that on an MR Diff, we can comment on any (even not changed) line in the left-hand side of the diff and only on changed lines in the right-hand side:

Update_test_js_take_2___2____Merge_Requests___Tomas_Vik___test_project___GitLab

When we have the commenting ranges, the user can click on the commenting gutter and start a comment:

Extension_Development_Host_-_test_js__MR____test-project

The buttons on the comment are custom defined and can have arbitrary functionality (Comment now, Add to review). This configuration is done in package.json and it is not officially documented.

GitLab API problem

We are not dogfooding our REST or GraphQL APIs for our code review on GitLab website. I wasn't able to create a new comment through our API in two days. Partly because I'm unfamiliar with the domain and partly because the documentation is quite brief. I faced several bugs (gitlab#281143 (closed) and one similar to gitlab#37066 (closed)) that I need to debug further before I can report them.

Without further investigation, I can't tell whether there is a way of implementing creating comments from VS Code or whether we need to implement/update/fix API endpoints first.

Known issues and uncertainty

  • The POC didn't implement adding comments and so there is large uncertainty.
  • I'm also not familiar with the MR Review workflows to be able to foresee any edge cases.
  • Large part of the comment functionality is undocumented and we might result in using vscode-pull-request-github as documentation (it's good that it is under MIT license).

Local git manipulation (e.g. Check out MR branch)

I wasn't able to include this in POC.

This page may contain information related to upcoming products, features and functionality. It is important to note that the information presented is for informational purposes only, so please do not rely on the information for purchasing or planning purposes. Just like with all projects, the items mentioned on the page are subject to change or delay, and the development, release, and timing of any products, features, or functionality remain at the sole discretion of GitLab Inc.

Edited by 🤖 GitLab Bot 🤖