Add support for decorating Pull Requests (Gitlab Server) #34

tisoft · 2019-09-21T20:03:17Z

Based on the work of @4n4n4s in #31 I've implemented the pull request decoration for gitlab. It's work in progress currently. Output can be seen here: https://gitlab.com/tisoft/sonarqube-community-branch-plugin-gitlab-test/merge_requests/1

Current features:

set Commit Status to succes/failed based on sonarqube quality status
add a summery comment
add line comments to files (still a bit buggy)
Deletion of old comments (configurable)

I will try to switch to conversations instead of notes/comments and see how they behave. Maybe both can be supported. Still playing around with this. 😄

mfolnovic · 2019-09-26T09:50:02Z

I think this is great to start with, I would probably implement deletion of old comments just because (as you can see from your examples), you'll get a lot of comments. :)

One thing I definitely miss is opening and closing discussions based on new sonar smells, which makes code review a lot easier. You could then:

see those comments during scrolling through all changes (which you can't see with just commit comments)
you can directly discuss if it's false positive / won't fix or if it should be fixed

But there are a bunch of corner cases so I'd rather do that separately. :)

tisoft · 2019-09-27T09:08:44Z

Deletion of old comments is already implemented. Will look into "real discussions" next week.

tisoft · 2019-10-11T06:48:59Z

Rebased against the changes from #31

mc1arke · 2019-12-26T19:54:08Z

@tisoft Could you rebase this against the updated base branch please?

tisoft · 2019-12-28T12:21:01Z

Merged in the new changes. Compiles and runs again.

I changed the visibility of findQualityGateCondition in AnalysisDetails to public, to get the new code coverage, since gitlab allows to submit that in commit status posts.

mc1arke

Thanks for your contribution!

Is there any reason you've made your changes on-top of Bitbucket decoration changes? It makes it a bit more difficult to review and unpick any issues.

Please add some tests to cover your new code. When we get to the point where I'm having to manage changes in Sonarqube and different providers (Gitlab, Bitbucket, Github, Azure, etc) APIs, I'm not going to have time to exhaustively check all the edge cases for each provider, so need there to be a high coverage of automated tests to do this.

For my comment on one of your classes and Json deserialisation, this also applies to other classes in your code, but I dind't want to spam the Pull Request, so you'll want to check all your JSON model classes.

Shout if you have any questions or disagree with my comments.

src/main/java/com/github/mc1arke/sonarqube/plugin/CommunityBranchPlugin.java

src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/AnalysisDetails.java

.../github/mc1arke/sonarqube/plugin/ce/pullrequest/gitlab/GitlabServerPullRequestDecorator.java

src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/gitlab/response/Commit.java

tisoft · 2019-12-30T13:36:05Z

@mc1arke thanks for the review! Will have a look into the issues next year🎇.

README.md

tisoft · 2020-01-02T10:46:18Z

Rebased against your latest base branch
removed bitbucket stuff
fixed some review comments

Where can I get the configuration settings, that are used by the "official" plugin, so that I can reuse the configuration names for those?

(Will add testcases as soon as the other review items are resolved)

mc1arke

My only major issue with this is the lack of unit or integration tests. I've not tried running it yet, but the code looks like it's doing what I'd expect.

src/main/java/com/github/mc1arke/sonarqube/plugin/CommunityBranchPlugin.java

src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/AnalysisDetails.java

src/main/java/com/github/mc1arke/sonarqube/plugin/CommunityBranchPlugin.java

mc1arke · 2020-01-02T15:45:03Z

SonarSource no longer seem to document the properties used for configuring each Pull Request provider. I'll see if I can confirm what they've used for Gitlab, but am happy you use the names you've suggested given they seem sensible.

mc1arke · 2020-01-06T19:24:35Z

The changes for Github and Bitbucket decoration have now been merged into the target pr-decoration branch. This has caused some conflicts against your changes in common files, and also needs updates in your changes for some constants that @4n4n4s created for the properties shared between your 2 changes. Could you rebase your MR and resolve these conflicts please?

tisoft · 2020-01-07T07:49:23Z

Rebased, will now cleanup and add tests

mfolnovic · 2020-01-07T09:22:13Z

.../github/mc1arke/sonarqube/plugin/ce/pullrequest/gitlab/GitlabServerPullRequestDecorator.java

+            LOGGER.info(String.format("Commits in MR: %s ", commits.stream().map(Commit::getId).collect(Collectors.joining(", "))));
+            List<CommitDiscussion> commitDiscussions = new ArrayList<>();
+            for (Commit commit : commits) {
+                getCommitDiscussions(projectURL + String.format("/repository/commits/%s/discussions", commit.getId()), headers, deleteCommentsEnabled)


what's the difference between creating discussions on MR's commits VS directly on MR?

there's an API for creating discussions or MR directly: https://docs.gitlab.com/ee/api/discussions.html#create-new-merge-request-thread

I am unsure about the difference between the 2 APIs. The one I'm using also creates a discussion, that can be replied to.

I will also try the MR discussion API out, to see if that has any differences. But I want to rebase, cleanup and test the current code before I do that.

cool 👍

what I would check is if discussion on commit shows up as thread in MR:

and if you turn on:

then you should see:

I don't see that with the commit comments.

I will play around with the MR discussions.

I have played around with MR discussions. code and result can be seen here: https://gitlab.com/tisoft/sonarqube-community-branch-plugin-gitlab-test/merge_requests/1

Resolving of discussions is currently not done by the plugin, it will delete all discussions and recreate them at each run.

What do you think?

nice, looks awesome, well done! 💯 💪

may I suggest adding a link to Sonar issue and/or Sonar rule? 👼

I would also try creating discussions first, and then creating MR comment with summary, so that would also be at the end (before user comments, of course). What do you think?

not sure if it's something on my end (tried also incognito), but images aren't loading for me:

other than that, looks really nice, thanks for trying this out! 😊

Personally I like having the summary on top, so that I have a quick overview and don't need to do endless scrolling before I can get to that.

Images are loaded from the github master branch of this repo, and since this is not yet merged into master, there are no images yet. The contents of the summary and issue comments is shared between all the decoration implementations, I don't want to change that as part as of this PR, but I agree that a link back to sonar would be helpful. Will do a followup PR as soon as everything else is settled.

If you click on the failed pipeline marker on the righrt and then on SonarQube, you are send to the sonar instance. (which will not work since its running on localhost on my machine 😆)

* set build status * add a summery comment * add file comments * delete all previous comments Notes: * Deleting comments will delete all comments from the commits made by the specified user, so this is only usefull if there is a dedicated sonar user configured in gitlab

mc1arke · 2020-01-07T20:53:09Z

Is this ready for final review and merge? If so, please remove the 'WIP' from the title.

tisoft · 2020-01-07T22:54:50Z

Its now ready from my point of view. I have changed it to use merge request discussions instead of file comments, since MR discussions look better in the gitlab merge request views.

mc1arke

2 very minor comments which it would be nice to fix if you get a chance, but wont block me merging this later today if you don't get round to it.

.../github/mc1arke/sonarqube/plugin/ce/pullrequest/gitlab/GitlabServerPullRequestDecorator.java

tisoft · 2020-01-08T08:58:39Z

fixed your comments in a new commit for easier review.

feel free to squash all commits into a single one.

don-vip · 2020-01-09T18:22:34Z

@tisoft how does the repository slug configuration works? I only see the parameter available in global administration settings and not in the project settings. How can I analyse several GitLab repositories with the plugin?

tisoft · 2020-01-09T19:53:41Z

I have it in the project settings:

don-vip · 2020-01-10T09:34:55Z

@tisoft thanks! I wasn't looking in the right place

don-vip · 2020-01-10T15:41:11Z

@tisoft des it work with SonarQube 8.0 or do we need 8.1? I can't trigger the MR decoration on my projects. I'm running a Maven build from GitLab CI with
mvn clean verify sonar:sonar $MVN_OPTS -Dsonar.pullrequest.key=$CI_MERGE_REQUEST_IID -Dsonar.pullrequest.branch=$CI_MERGE_REQUEST_SOURCE_BRANCH_NAME -Dsonar.pullrequest.base=$CI_MERGE_REQUEST_TARGET_BRANCH_NAME

The branch analysis is performed, but nothing happens concerning the decoration. I have configured GitLab host and acces token in global settings, project slug in project settings, and sonar.pullrequest.* parameters on the command line. Am I missing something? In TRACE level I can see this in ce.log:

2020.01.10 16:06:24 INFO  ce[o.s.c.t.p.a.p.PostProjectAnalysisTasksExecutor] Webhooks | globalWebhooks=0 | projectWebhooks=0 | status=SUCCESS | time=15ms
2020.01.10 16:06:24 DEBUG ce[c.g.m.s.p.c.p.PullRequestPostAnalysisTask] found 3 pull request decorators
2020.01.10 16:06:24 TRACE ce[c.g.m.s.p.c.p.PullRequestPostAnalysisTask] Current analysis is not for a Pull Request. Task being skipped
2020.01.10 16:06:24 INFO  ce[o.s.c.t.p.a.p.PostProjectAnalysisTasksExecutor] Pull Request Decoration | status=SUCCESS | time=0ms

EDIT: I've found the problem, my command line also contained the parameter sonar.branch.name which conflicts with the sonar.pullrequest.* parameters. Removing it solved the issue.

This was referenced Sep 21, 2019

Add support for decorating Pull Requests #30

Merged

Decorating Pull Requests #27

Closed

tisoft force-pushed the pr-decoration-gitlab branch from cdc5117 to fa0b186 Compare September 23, 2019 09:59

tisoft force-pushed the pr-decoration-gitlab branch from fa0b186 to 872fadf Compare October 11, 2019 06:48

kayoub5 mentioned this pull request Nov 30, 2019

SonarQube 7.7+ support gabrie-allaigre/sonar-gitlab-plugin#241

Open

mc1arke force-pushed the pr-decoration branch from f053ed7 to 3eb5def Compare December 20, 2019 07:50

mc1arke requested changes Dec 29, 2019

View reviewed changes

mc1arke force-pushed the pr-decoration branch from 3eb5def to e808a9d Compare December 30, 2019 13:21

mc1arke force-pushed the pr-decoration branch from e808a9d to ce99b08 Compare December 30, 2019 16:23

mc1arke reviewed Dec 30, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

tisoft force-pushed the pr-decoration-gitlab branch from d94425a to 807e3cd Compare January 2, 2020 10:43

tisoft force-pushed the pr-decoration-gitlab branch 2 times, most recently from 3114ac2 to 42ce3bd Compare January 2, 2020 10:55

tisoft requested a review from mc1arke January 2, 2020 10:58

tisoft force-pushed the pr-decoration-gitlab branch 3 times, most recently from f84dacf to 6ebd6e0 Compare January 2, 2020 15:31

mc1arke requested changes Jan 2, 2020

View reviewed changes

tisoft force-pushed the pr-decoration-gitlab branch 2 times, most recently from beeac00 to 7a9fceb Compare January 7, 2020 07:43

mfolnovic reviewed Jan 7, 2020

View reviewed changes

tisoft force-pushed the pr-decoration-gitlab branch 2 times, most recently from a115f77 to 6d008a0 Compare January 7, 2020 11:02

tisoft force-pushed the pr-decoration-gitlab branch from 6d008a0 to 659a7e5 Compare January 7, 2020 17:04

Use Merge Request discussions for file comments

1e224d8

tisoft changed the title ~~WIP: Add support for decorating Pull Requests (Gitlab Server)~~ Add support for decorating Pull Requests (Gitlab Server) Jan 7, 2020

mc1arke approved these changes Jan 8, 2020

View reviewed changes

.../github/mc1arke/sonarqube/plugin/ce/pullrequest/gitlab/GitlabServerPullRequestDecorator.java Outdated Show resolved Hide resolved

.../github/mc1arke/sonarqube/plugin/ce/pullrequest/gitlab/GitlabServerPullRequestDecorator.java Outdated Show resolved Hide resolved

cleanup

6e33b32

mc1arke approved these changes Jan 8, 2020

View reviewed changes

mc1arke merged commit e1894f0 into mc1arke:pr-decoration Jan 8, 2020

tisoft deleted the pr-decoration-gitlab branch January 8, 2020 21:43

Dzamir mentioned this pull request Feb 19, 2020

The settings for the Pull Request are not present #95

Closed

placaze mentioned this pull request Mar 11, 2020

[GitLab][Pull Request Decoration] Baseline of Sonarqube/Plugin versions & configuration info #118

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for decorating Pull Requests (Gitlab Server) #34

Add support for decorating Pull Requests (Gitlab Server) #34

tisoft commented Sep 21, 2019 •

edited

Loading

mfolnovic commented Sep 26, 2019

tisoft commented Sep 27, 2019

tisoft commented Oct 11, 2019

mc1arke commented Dec 26, 2019

tisoft commented Dec 28, 2019

mc1arke left a comment

tisoft commented Dec 30, 2019

tisoft commented Jan 2, 2020 •

edited

Loading

mc1arke left a comment

mc1arke commented Jan 2, 2020

mc1arke commented Jan 6, 2020

tisoft commented Jan 7, 2020

mfolnovic Jan 7, 2020

tisoft Jan 7, 2020

mfolnovic Jan 7, 2020

tisoft Jan 7, 2020

tisoft Jan 7, 2020

mfolnovic Jan 7, 2020

tisoft Jan 7, 2020

mc1arke commented Jan 7, 2020

tisoft commented Jan 7, 2020

mc1arke left a comment

tisoft commented Jan 8, 2020

don-vip commented Jan 9, 2020

tisoft commented Jan 9, 2020

don-vip commented Jan 10, 2020

don-vip commented Jan 10, 2020 •

edited

Loading

Add support for decorating Pull Requests (Gitlab Server) #34

Add support for decorating Pull Requests (Gitlab Server) #34

Conversation

tisoft commented Sep 21, 2019 • edited Loading

mfolnovic commented Sep 26, 2019

tisoft commented Sep 27, 2019

tisoft commented Oct 11, 2019

mc1arke commented Dec 26, 2019

tisoft commented Dec 28, 2019

mc1arke left a comment

Choose a reason for hiding this comment

tisoft commented Dec 30, 2019

tisoft commented Jan 2, 2020 • edited Loading

mc1arke left a comment

Choose a reason for hiding this comment

mc1arke commented Jan 2, 2020

mc1arke commented Jan 6, 2020

tisoft commented Jan 7, 2020

mfolnovic Jan 7, 2020

Choose a reason for hiding this comment

tisoft Jan 7, 2020

Choose a reason for hiding this comment

mfolnovic Jan 7, 2020

Choose a reason for hiding this comment

tisoft Jan 7, 2020

Choose a reason for hiding this comment

tisoft Jan 7, 2020

Choose a reason for hiding this comment

mfolnovic Jan 7, 2020

Choose a reason for hiding this comment

tisoft Jan 7, 2020

Choose a reason for hiding this comment

mc1arke commented Jan 7, 2020

tisoft commented Jan 7, 2020

mc1arke left a comment

Choose a reason for hiding this comment

tisoft commented Jan 8, 2020

don-vip commented Jan 9, 2020

tisoft commented Jan 9, 2020

don-vip commented Jan 10, 2020

don-vip commented Jan 10, 2020 • edited Loading

tisoft commented Sep 21, 2019 •

edited

Loading

tisoft commented Jan 2, 2020 •

edited

Loading

don-vip commented Jan 10, 2020 •

edited

Loading