Quality Gate + Short Lived Branches

I fully agree with the statement from Joe Swanson.

Our SW architects complained about the PR annotation and I disabled this feature for the current milestone activities. I really would like to see an issue filter for rules or severity.

2 Likes

Hi guys,

I find it unfortunate that this old topic was resurrected with such an aggressive tone. Maybe it’s not easy to find if you don’t know the right key words, but this question has been asked - and answered - several times in this community in the last few months:

To be explicit, each of these threads references

MMF-1369 - Real Quality Gates for PRs and short-lived branches

The goal of which is to enable you to “make [your] own decisions about what quality is acceptable to [you] for all branches, short or long term”. Feel free to watch and vote for this initiative, which is currently tentatively scheduled for 2019Q1.

In the meantime, yes there is a hard-coded gate on PRs and Short-Lived Branches. At initial implementation our choice was doing that or doing nothing. We picked the former as the lesser of two evils, especially since we don’t impose a zero leak policy, we just make sure you’re aware of what you’re doing when you add issues in the leak. As described above,

To be super explicit about this, if your PR has no Status=Open issues, then it goes green, even with non-fixed issues. And it doesn’t just go green in SonarQube; PR annotation is updated as well when the last Open issue is moved to some other status. So, to not be blocked by “a draconian zero leak policy that cannot be modified”, simply “Confirm” all your Open issues.

If you’d like a civil discussion of other pains you have around this topic, please feel free to start a new thread.

 
Ann

6 Likes

I read through this thread because it came through on the digest. Nowhere do I see an aggressive tone: I’m not sure where you are getting that idea.

I too fought with this issue on one of the previous threads you are mentioning. And I eventually did find the documentation that has been pointed out here. I also remember that it was not easy to find this explanation. Personally, I think the reason is that the workflow SonarQube has in their mind is somewhat… unique. (Which causes one to not look in the right places, because we aren’t using the same logic.) It really doesn’t jive with any workflow I’ve been a part of. Now that I understand it, we work with/around it, but the most aggressive thing I’ve seen is SonarQube’s stubbornness to realize they may have missed the use case here, and instead, are standing firm.

I’m still not sure why we would have Quality Gates for after the fact analysis when the default “before the fact” analysis is for all 0’s. Unlike what was mentioned earlier, that seems that would only work for projects that are not greenfield. But even then, how could a Quality Gate get worse if you have to fix or mark everything with some type of resolution?

2 Likes

Hi,

Could you clarify which documentation you’re talking about here? And where/how you think it would be more obvious to put it?

If by this you’re referring to the red/green status on SLBs and PRs, we’ve said pretty much from the beginning that we want to get to the place where users can control what tips it one way or the other.

That’s an excellent question, and one we’ve asked internally as well. When we do introduce user-controlled Quality Gates for SLBs and PRs it’s possible we’ll take action on it, but right now that would probably be premature.

What is your understanding of what happens to issues that are marked Confirm in a PR?

 
Ann

1 Like

Hi Ann,

Thank you for the summary and the MMF-1369 reference.

Unfortunately the scope of our quality profiles were created without the 0 issue Quality Gate policy in mind and I really want PR analysis with annotations enabled for all of our projects.

I will refine our quality profiles for PRs and use it as default quality profile. I will remove all rules which have severity “INFO”, “MINOR” because we think comments for this kind of issues shall be avoided.
Nevertheless we want to see the issues for “INFO”, “MINOR” severity rules for our code base every day but not for pull request analysis.

How can I achieve this? I cannot define a second quality profile which is used for our nightly builds. The project configuration has only one quality profile per language.

Maybe there is a configuration which I am not aware of.

Best regards
Bert

Hi Bert,

Unfortunately you’re not going to be able to use a different QP for your PRs & SLBs.

 
Ann

And where/how you think it would be more obvious to put it?

This is a difficult question, as the manual sections are influenced by the workflow, and the workflow really doesn’t jive with me. Maybe in the Overview section put a diagram of the designed-to workflow would then help people enter further into the documentation.

What is your understanding of what happens to issues that are marked Confirm in a PR?

My understanding has expanded based upon your previous answer in this thread, and I think it confirms my assertion of the awkwardness of this workflow. What you describe as ‘Confirm’ sounds like it would be worded something like ‘Absorb as technical debt to be resolved in the future.’ Confirm, on face value, without explanation, doesn’t imply that at all.

When my team was presented with the choices:

  • “Confirm”
  • “Resolved as False Positive”
  • “Resolved as Won’t Fix”
  • “Assign”
  • “Comment”
    it was very confusing. They don’t seem mutually exclusive, confirm doesn’t seem related to the next two (now we know why), and we are left wondering how to properly use them. As far as I can tell, this is attempted to be explained in Pull Requests and Issues. But the two pages use different terminology and does not clarify proper actions to the reader.

My take away is that SonarQube doesn’t seem to trust itself and wants a human to either tell it, yes, you are right (confirm) or no, you are wrong (false positive) or yes, you are right, and I don’t care (won’t fix) or yes, you are right, I’m going to go fix that right now (assign). I would think SonarQube trusts that it is doing its job unless I tell it otherwise… the value of “Confirm” is unclear.

1 Like

Hi,

I suppose our expectation/understanding of the “Confirm” option is an outgrowth of our history using SonarQube. In that context, “Confirm” has always meant just what you describe: “Absorb as technical debt to be resolved in the future.”

We have had some internal discussions about renaming it, but couldn’t come up with anything both terse and accurately descriptive.

I’d say your characterizations of Won’t Fix and False Positive are correct. For assign - which to be clear is independent of for example choosing Confirm in the Actions menu - whether the fix is immediate or not is about working agreements within a team. By default issues should be assigned to their creators* so explicit assignment shouldn’t generally be needed.

For Confirm the meaning is really “yes this is an issue, but I’m not going to fix it now”. It’s not a question of SonarQube trusting itself, but of whether you as a developer/team want to fix the issue before merge or move forward despite its presence. To flip the PR status to green despite having non-fixed issues in the code, you Confirm the remaining issues to acknowledge them and indicate that they’re acceptable for now.

* Corner cases remain

 
Ann

1 Like

Ann, I too came across this thread because of the digest notification. Without getting into the issues surrounding quality gates for PRs, I would echo the comments regarding the documentation about the impact of confirming an issue. Until I read this thread, I had no idea that confirming an issue would count toward eliminating it as an open issue for the PR. I think the problem is that since the issue is still “open” (i.e. not resolved), it’s not clear that this resolution removes it from the open issue count for the PR. I’m guessing that’s documented somewhere, but it might be worth explicitly calling it out in the PR decoration docs.

1 Like

Hi,

In the context of PR and SLB status, “Open” refers specifically to the issue status rather than whether it’s fixed or not in code.

Re documentation, FYI (emphasis added):

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.

From Pull Request analysis

Also (emphasis added)

It is possible to change the status of a short-lived branch from ERROR to OK (red to green), i.e. mergable, by manually confirming the issues. The same is true for the False-Positive and Won’t Fix statuses. It means the status of a short-lived branch will be red only when there are Open issues in the branch.

From https://docs.sonarqube.org/latest/branches/short-lived-branches/

For easy reference, you can find these docs embedded in your SonarQube instance under the ‘?’ in the main menu. Some docs are available before 7.4, but with 7.4 we completed the port of documentation into SonarQube itself.

 
Ann

1 Like

You’re right. TBH I don’t know why I missed it before.

2 Likes

Hi,

We’ve got a slow-moving, but on-going initiative to embed relevant bits of documentation in the UI. You’ve made me realize this is a good candidate, so I’m going to see what I can get added for 7.5. Hopefully by 7.6 we’ll have real quality gates (no promises!) and this becomes moot.

 
Ann

The down side of this particular approach is that it actually generates a significant amount of churn in shops with fully/heavily automated CI/CD systems. The reason I found this post to begin with was because I was trying to resolve issues with Jenkins failing jobs because the build is failing due to the quality gates. So the reality of my situation is that developers push a commit, jenkins kicks off the build pipeline, sonarqube fails the quality gate because someone has an informational issue in the code (that would have been allowed to leak by policy, not manual exception), now a developer has to go into sonar and see what’s up (which they don’t have to do normally because they get the reports linked through IntelliJ and may not actually have access to sonar since there is usually no need), mark the items appropriately, and then trigger a new build so that the quality gate can pass.

I appreciate that there is a work around for the issue. It is unfortunate that it is a manual work around that impedes automation and policy enforcement, and diverts resources from positive business value areas. It just seemed like a significant step backwards for such a wonderful product.

2 Likes

Hi,

Just to clarify

The quality gate flips to passing as soon as you do something with the last Open issue.

If your problem is not the quality gate but the build… well, we’ve said for a long time that breaking the build because of quality issues isn’t the best strategy. We nonetheless added webhook notifications so you can notify external systems of your project’s QG status. In addition, we fire the webhook when manual issue changes flip the QG status.

Thanks. I think. :smile:
Just to repeat one more time, we recognize this is sub-optimal and plan to get to the point of offering real, user-controlled quality gates on PRs and SLBs. We’ve delivered interim stages of the product because we believe in delivering, rather than waiting until we’ve got a theoretically “feature-complete” version.

 
Ann

1 Like

A post was split to a new topic: Short Lived Branch status in 6.7.4

Agreed, i would like to delete sq right now -__- our checks are worth nothing right now

Is there some where I can vote this feature up? This would be a big win for my team. I would happily provide any feedback to explain my use case.

This feature is currently in development @Alex_Denton and should be included in 7.7.

 
Ann

Glad to hear it! Keep up the good work.

1 Like

@ganncamp I couldn’t find this feature on Sonarcloud yet and also the documentation still states about this limitation. Was this part of 7.7 or is it delayed? And what is the plan about this feature then?