Support commenting on the diff

[tomasbjerre]

Waiting for: https://gitlab.com/gitlab-org/gitlab-ce/issues/14850

[bonzani]

It seems now possible to start a discussion on a specific line in the diff with the position[new_line] and position[old_line] attributes. https://docs.gitlab.com/ce/api/discussions.html#create-new-merge-request-discussion

[tomasbjerre]

This plugin is using this library: https://github.com/timols/java-gitlab-api

This functionality needs to be added in that library.

[bonzani]

I have added the functionality with timols/java-gitlab-api#313 It should be included in the next release of the library.

[tomasbjerre]

Nice!

Are you thinking one new "Discussion" per violation? I'll have to investigate this a bit. Or maby you want to implement the feature here as well?

[bonzani]

Yes I think for each violation in the diff one new discussion should be created with the violation as a body. I will create a PR as soon as I can make it work.

[tomasbjerre]

Ok. It might be convenient to change the dependency, in the lib, on java-gitlab-api like this during development:

compile 'com.github.timols:java-gitlab-api:master'

To get it using Jitpack.

[tomasbjerre]

Some more notes:

• shouldCreateSingleFileComment needs to be implemented with an attribute: https://github.com/tomasbjerre/violation-comments-to-gitlab-lib/blob/master/src/main/java/se/bjurr/violations/comments/gitlab/lib/GitLabCommentsProvider.java#L191
  • createSingleFileComment needs to be re-implemented. It is just an old experiment now: https://github.com/tomasbjerre/violation-comments-to-gitlab-lib/blob/master/src/main/java/se/bjurr/violations/comments/gitlab/lib/GitLabCommentsProvider.java#L103
  • And to understand what this is, here is where it is invoked: https://github.com/tomasbjerre/violation-comments-lib/blob/master/src/main/java/se/bjurr/violations/comments/lib/CommentsCreator.java#L59

[tomasbjerre]

Did some adjustments in a feature branch: https://github.com/jenkinsci/violation-comments-to-gitlab-plugin/tree/feature/comment-on-diff

[tomasbjerre]

@bonzani I'm getting comments like this. Was that the intention?

I was expecting the comments to be on the diff, like this:

[tomasbjerre]

I found this: https://docs.gitlab.com/ee/api/discussions.html#create-new-merge-request-discussion

Perhaps part of the solution is supplying "type":"DiffNote", but my comments still show up as just ordinary comments.

[bonzani]

I was expecting that too. I digged into the problem and found, that for the discussion to be of type DiffNote, the following arguments have to be supplied: body, position[new_line], position[position_type], position[new_line], position[old_line], position[new_path], position[old_path], position[base_sha], position[start_sha], position[head_sha] I think at the new_path, old_path, old_line parameters need to be set correctly. I will create a PR as soon as I have some free time to spend on it.

[tomasbjerre]

I started fiddling with it with a script here: https://github.com/tomasbjerre/violation-comments-to-gitlab-lib/tree/master/sandbox

[bonzani]

I made a few adjustments to your sandbox script in the PR such that it will work.

[tomasbjerre]

Great!

[tomasbjerre]

I can set old path and new path correctly now. But 2 things is yet to solve:

• Setting oldLine correctly.
• Posting it as query params.

[bonzani]

I need to think about a solution for the oldLine problem, but I created timols/java-gitlab-api#323 to solve the query params issue.

[tomasbjerre]

Perhaps it is possible from parsing the diff. I did not have time to finish it. https://github.com/tomasbjerre/violation-comments-to-gitlab-lib/commit/ecdb925a1f50801ab7710b1e15b4b47c3889ffe3

[bonzani]

Nice, I will look into it next time I have some spare time. Just that it does not get lost: When a new line is added, the oldLine parameter needs to be null (then it does not get added to the request).

[tomasbjerre]

I found a way to get oldLine from the diff: https://github.com/tomasbjerre/violation-comments-to-gitlab-lib/blob/master/src/main/java/se/bjurr/violations/comments/gitlab/lib/GitLabCommentsProvider.java#L122 https://github.com/tomasbjerre/violation-comments-lib/blob/master/src/main/java/se/bjurr/violations/comments/lib/PatchParser.java#L43

[tomasbjerre]

I got it working now with the Jenkins plugin!

[bonzani]

Very nice! I am happily looking forward to use it in the near future.

[tomasbjerre]

Released in 2.12 now.