Open baaym opened 6 years ago
Hi, Thanks, I look at what you did.
Hi @gabrie-allaigre,
I tested this on our SonarQube instance, and unfortunately there's an error I will have to look at:
[INFO] Executing post-job GitLab Commit Issue Publisher
[WARNING] Property 'sonar.gitlab.commit_sha' is not declared as multi-values/property set but was read using 'getStringArray' method. The SonarQube plugin declaring this property should be updated.
[ERROR] Failed to execute goal org.sonarsource.scanner.maven:sonar-maven-plugin:3.4.0.905:sonar (default-cli) on project xxx: SonarQube failed to complete the review of this commit: Issues are only available to PostJobs in 'issues' mode. -> [Help 1]
I think the warning is the result of some other changes in the master branch.
Regards, Martijn
Hi,
In
PropertyDefinition.builder(GITLAB_COMMIT_SHA).name("GitLab Commit SHA").description("The commit revision for which project is built.").category(CATEGORY) .subCategory(SUBCATEGORY).index(6).hidden().build()
Add .multiValues(true)
Hi @gabrie-allaigre,
I reverted a lot of changes since I found out that the check for new issues was not done from the CommitPublishPostJob
, but inside ReporterBuilder.getStreamIssue()
. I think I got confused due to the sonarFacade.getNewIssues()
call in the CommitPublishPostJob
. This method doesn't get new issues at all, it gets all issues. Filtering happens later inside the ReportBuilder. Perhaps it's good to change the name of SonarFacade.getNewIssues()
?
The merge request should be complete now. I will test these changes in the office on Monday, and will let you know what comes out. After that, and if the merge request is approved, I think it's a good idea to squash all these commits ;)
Hi @gabrie-allaigre,
The plug-in runs without errors or warnings now. However, the sonarFacade.getNewIssues()
method returns 0 issues. I see it was recently changed in Master when updating to SQ 7.0. Perhaps a bug was introduced?
Hi,
sonarFacade.getNewIssues()
return issues for ref_name branch only.
Perhaps that's the issue. I think in the Community Edition of SonarQube the branch identifier is deprecated since 6.6. See: https://docs.sonarqube.org/display/SONAR/Analysis+Parameters#AnalysisParameters-ProjectConfiguration.1
That means that projects that supply a sonar.branch
property are probably not registered as that branch. My projectname is automatically appended with the branch name (develop
), however the actual branch identifier in the GUI shows master
.
Perhaps the plugin should not rely on the actual GitLab branch name to do the searching in SonarQube. Does the SonarQube API provide info on which SonarQube branch is currently being analysed?
Hi @gabrie-allaigre,
I'm abandoning this pull request. I made a change internally so the proper branch was selected. Unfortunately the results coming from the search in publish mode deviate too much from the results coming from the preview mode. I think that if one wants to use SonarQube to generate CodeClimate files for GitLab EE, only preview mode should be used on all branches.
The changes in this pull request can still be useful for others though, so feel free to merge anyway.
Hello!
I recently started using this plugin on our GitLab EE instance, and in general it works like a charm. When I configured the CodeClimate report, things were less smooth unfortunately.
On our main branch we have SonarQube running in publish mode. In publish mode, this plugin only handles issues that are reported as new by SQ. This means that in our main branch the CodeClimate file never contains all issues. In preview mode, which is being used by merge requests, all issues are correctly reported.
GitLab compares both CodeClimate files when showing the merge request window. So when the full CodeClimate file in the merge request is compared to the near-empty one in the main branch, you can imagine the shock ;)
The goal of this merge request is to add support for writing all issues to the CodeClimate file, regardless of operating mode. I tried to do this by introducing a new property, called
sonar.gitlab.json_all_issues
. Basically all behavior is the same, except when you set that property totrue
and you run in publish mode. In that case you will get all issues in the JSON file, while still reporting only new issues as comments.Unfortunately this required some poking around in unfamiliar code. There are a few things I'm not really proud of - for example the
Reporter.build()
method signature change to name one. Perhaps it's not all that bad, that's for the maintainers to judge ;)Please let me know what you think!
Regards, Martijn