Pre-view analysis with quality gate in place for PRs?

[hbal...@gmail.com]

Hi,

I'd like to support a pull-request build flow, where the project specific quality profiles and quality gate is used to determine the outcome. In other words:

I want the analysis to break the pull request, if the merged project would break.

I realise I have the BitBucket plugin and SonarLint Cli providing pre-view functionality, but with these the main issue is lack of quality gate support. Breaking on a simple rule like the severity of issues found is something I'd like to avoid.
Optionally the analysis should only consider the code changed by the PR. In this case the analysis could still succeed if there are otherwise unfixed issues.

Is there such a solution available?

Thanks and Cheers,
Balázs

[G. Ann Campbell]

Hi Balazs

(Sorry, my keyboard doesn't have an accented 'a')

There's nothing currently available to support this use case, and we don't have plans to add anything.

Why? Well, it's not always possible to code with 0 issues. Sometimes you have to take one step back to take two steps forward. Breaking a pull request (I'm not even sure what that actually means) removes the element of human judgement from PR review, and that's not a concept we support - either technically or ideologically.

Ann

[Balázs Hosszu]

Dear Ann,

Thanks a lot for your answer, and don't worry about the accent. :)

By breaking the pull request, I mean to include a build step (may it be Ant or Gradle or your favourite build tool), that triggers the Sonar analysis on the project. This build job fails, if the Sonar analysis ( according to the quality gate) fails. For a standard analysis this is already possible trough the BuildBreaker plugin [1]. The tricky part would be getting the same end-result without overwriting the project results with the possibly temporary results of the pull-request contents.

I agree with you on the importance of the human judgement, my idea was to move that work into commonly agreed quality profiles and a quality gate. Strict at code-review time, requiring a continuous team agreement and rule alignment. I don't see this as breaking the concept of the waterleak model, which means to me it's okay to have issues in your backlog, concentrate on the meaningful, on the new stuff. Your answer is well appreciated and I will consider it!

I see a technical issue with your workflow. Assuming the situation: I can't decide if an issue would break the quality gate or not before committing it, neither in development nor in review time. If such an issue looks good to all humans involved, it will be merged. With the next full Sonar analysis this issue will break the quality gate. You are suggesting the humans should not have decided the validity of that code based on the quality gate. I think this implies the quality gate as a whole concept should not be taken too strongly: issues should be evaluated case-by-case, and based on some general project wide rules.

What do you think is the value of the quality gate, if at either development or review time I should not consider it?

Much appreciated,
Balázs

[1] : https://github.com/SonarQubeCommunity/sonar-build-breaker

[G. Ann Campbell]

Hi Balazs,

I get what you're saying, but I think you're throwing the baby out with the bathwater.

So let's say I submit a PR and it does introduce a new issue, which is raised by PR analysis. The human reviewer sees it and brings it up to me. I either fix it, or explain why I think the issue needs to be merged into master anyway. 

Let's take as an example something I've actually faced: the use of System.out.println. There's a rule about not doing that, and for the vast majority of applications it's appropriate. But I happen to be working on a console application, so System.out.println should be acceptable. So... now my reviewer and I face a few choices:

* remove the rule from the profile - bad because most projects need this rule

* set up exclusions so this rule isn't applied to my file - this is an okay option but maybe overkill

* introduce a logging framework to write to do the the console output - this would work seems impractical 

* let the issues be raised and mark them Won't Fix in the platform

What I actually did in this situation was choose the last option, but if I had somehow not been able to merge the PR, I'd have been forced to a less attractive option.

So the premise here is that sometimes you have to introduce issues, and either they're temporarily acceptable and you'll fix them soon (hopefully before release), or they're more permanently acceptable and you'll take some other measure. 

None of this invalidates the concept of the Quality Gate. The project should still be passing at release - or there should be a very good reason why it's not, and probably a remediation plan in place.

And just as a side note, we don't advocate a 0-issues QG when it comes to Code Smells. Zero new bugs, sure. Same for vulnerabilities. But the default Quality Gate looks at the ratio of new Code Smells to acceptable code.

Ann

---

G. Ann Campbell | SonarSource

Product Manager

@GAnnCampbell

http://sonarsource.com

--
You received this message because you are subscribed to a topic in the Google Groups "SonarQube" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/sonarqube/fodF3FZAHQY/unsubscribe.
To unsubscribe from this group and all its topics, send an email to sonarqube+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/d6bf518c-d322-4949-9c1d-63fa0dff4658%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Balázs Hosszu]

Hi Ann,

Thanks for taking the time to answer!

Your example was very helpful actually. In that situation I'd have deemed excluding the rule from the project, or creating a specific profile the correct way to go.
Perhaps being this strict could be helpful in maintaining a continuous delivery, where theoretically every commit must be quality-gate level good, but overall this might be an overshoot for standard development.

Thanks and cheers,
Balázs

To unsubscribe from this group and all its topics, send an email to sonarqube+...@googlegroups.com.