[FOLIO-1617] SonarCloud Integration now fails pull requests on quality gate checks Created: 15/Nov/18  Updated: 03/Jun/20  Resolved: 30/Nov/18

Status: Closed
Project: FOLIO
Components: None
Affects versions: None
Fix versions: None

Type: Task Priority: P3
Reporter: Marc Johnson Assignee: John Malconian
Resolution: Done Votes: 0
Labels: core, devops, sprint51
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original estimate: Not Specified

Attachments: PNG File Screenshot 2018-11-15 15.30.48.png     PNG File Screenshot 2018-11-15 16.15.14.png     PNG File Screenshot 2018-11-20 12.24.39.png    
Sprint:

 Description   

It seems the new SonarCloud integration fails the check when the pull request changes fail the quality gate for the module.

This does not block the pull request from being merged, so this is not a critical issue.

Looking at the documentation

When "Confirm", "Resolved as False Positive" or "Won't Fix" actions are performed on issues in SonarQube UI, the status of the PR is updated accordingly. This means, if you want to get a green status on the PR, you can either fix the issues for real or "Confirm", "Resolved as False Positive" or "Won't Fix" any remaining issues available on the PR.

I think that Sonar have made the decision that issues identified in analysis have to be deliberately attended to, and we either accommodate that, lower the standards, or turn off analysis completely on pull requests in order for that check to pass.

As using the sonar cloud UI requires setting up a relationship with each github user (and that adds a big management overhead), if we want to accommodate this in our process, we may need to restrict overriding issue resolution to a limited set of developers.



 Comments   
Comment by John Malconian [ 15/Nov/18 ]

So there have been a few changes to Sonarcloud recently. The Github integration is a bit different, but the exciting news is that we have the ability to create custom quality gates if we don't want to use the Sonar default quality gate. Some ideas and thoughts:

  • It's important to note that Sonarcloud cannot block the ability to merge a pull request. This is something that is configured in Github on a per repository basis. We currently have not set this on any repositories.
  • We have both the ability to create custom "Quality Gates" in Sonarcloud as well as the ability to modify "Quality Profiles" - that is, the ability to tweak rules and create custom profiles individual rules priority levels can be changed. i.e . changing 'major' to 'minor', etc or removed all together.
  • I think it may be a good idea to create a Sonarcloud team that consists of a subset of frontend and backend FOLIO developers that have administrative access to our Sonarcloud account and who have the ability to tweak rules, profiles, and quality gates more suitable to our needs. For instance, if the existing 'cognitive complexity' rules in the Java profile don't work for us, then we can tweak those rules across all projects so that they do not need to be addressed individually. I would keep this team relatively small - perhaps no more than six developers.
Comment by Marc Johnson [ 20/Nov/18 ]

It would appear the SonarQube integration fails the check (without blocking the pull request) when it discovers any issues, see https://sonarcloud.io/project/issues?id=org.folio%3Amod-circulation&pullRequest=152&resolved=false as an example (screenshot attached for when link stops working)

Comment by John Malconian [ 20/Nov/18 ]

As long as it doesn't block the PR from being mergeable, I think I can live with that Marc Johnson unless you think it's a problem.

Comment by Marc Johnson [ 20/Nov/18 ]

John Malconian Agreed, given that it does not block the pull request from being merged, I think conversations about this (and general SonarQube policy) can be deferred.

we briefly raised it in the core team backend technical meeting, mostly to raise awareness that people may see more failed checks than before.

Comment by John Malconian [ 28/Nov/18 ]

Marc Johnson Can I go ahead a close this or is there anything outstanding we need to address?

Comment by Marc Johnson [ 30/Nov/18 ]

John Malconian I think this can be closed.

I don't think we can particularly change the integration behaviour at this point (and in general it is a big improvement).

I think we may need to accept that there will be more pull requests that get merged despite a red cross, and be mindful of not ignoring the sonar checks because of this.

Generated at Thu Feb 08 23:14:39 UTC 2024 using Jira 1001.0.0-SNAPSHOT#100246-sha1:7a5c50119eb0633d306e14180817ddef5e80c75d.