Skip to content

MR review: new comments and reviews POC

Problem to solve

This issue is a continuation of the #269 (closed) investigation. The #269 (closed) investigated how te work with existing comments and this POC is validating implementation for creating new comments and managing reviews.

We had a few issues when integrating with the GitLab API(gitlab#281143 (closed), gitlab#282476 (closed), gitlab#282481). Part of the validation is going to be checking that all GitLab endpoints are now working as expected.

Proposal

Create POC that answers the following questions:

  • How are we going to create a comment? What's going to be the VS Code behaviour if we fail to send the comment (on the web the text area remembers the comment)
  • How are we going to contribute to a thread?
  • How can we represent MR review in progress? What is the API for showing unsubmitted reviews?
  • How can we edit and delete existing comments?
  • What happens when we submit a review that is out of date? In other words: somebody committed changes since we started the review.

Result

new_comments

Try the POC extension: gitlab-workflow-4.0.0.vsix

The POC is implemented in a branch: 293-new-comments-poc

TL;DR

Creating new comments is possible, however, there are several API deficiencies[1][2] that will make extension implementation harder requiring several workarounds. Fixing the deficiencies would simplify the implementation a lot.

Overview of the work:

  • MR Review: Respond to a diff thread
    • relatively simple because we don't need to pass in a position for the new note, we'll just send discussion.replyId with the request to explain what we are responding to
    • the only issue that can't be made simpler by fixing the GraphQL API
  • MR Review: add commenting ranges
    • We need to compute what lines have been added, based on text diff hunks that we get from the API.
    • We then use this information to show commenting ranges (gutters) that allow users to comment on certain lines.
  • MR Review: track changed lines on the diff
    • This issue is to track the line index of an unchanged line. The unchanged line can be "pushed down" or "pulled up" relative to the new version of the diff and we must track that difference to be able to submit a comment on unchanged line.
    • There are some unnecessary constraints introduced by the API that make it extra hard for us the create notes on unchanged lines[1]. If we fix [1], we don't have to implement this issue.
  • MR Review: Introduce global MR cache
    • thanks to [1] and [2] we need much more information for creating new notes than what can fit in the URI (which VS Code uses to communicate between the extension and the diff view).
    • we need to introduce a global cache so we can easily get all the MR information just by knowing its ID
    • It would be possible to get away without implementing this issue if [1] and [2] get fixed, but I think it's still worth simplifying the way we handle MR information (GitHub Pull Request does something similar). But if [1][2] get fixed, this is not a blocker.
  • MR Review: create a new comment on the diff
    • This is the final issue, it's going to be quite a lot of effort and has got weight of 4
    • We'll have to implement a workaround for [2]

Dependencies

Only the last issue (MR Review: create a new comment on the diff) is dependent on all the previous issues.

graph LR
A["Respond to a diff thread"] --> D["create a new comment on the diff"]
B["add commenting ranges"] --> E["track changed lines on the diff"]
E --> D
C["Introduce global MR cache"] --> D

[1]: GraphQL createDiffNote forces clients to compute redundant information

[2]: GraphQL createDiffNote mutation doesn't fully support interactive clients

Edited by Tomas Vik