Quality Gate + Short Lived Branches

q_gate
branches
sonarqube

(Gagan) #1

Hi

I wanted to confirm quality gate behaviour for short-lived branches. It seems the short-lived branches do not adhere to the Quality Gate assigned to a project. Do the quality gate profiles only apply to long-lived branches?

For example, a Quality Gate for one of my projects contains a rules to throw warnings instead of a errors. However the sonar quality gate in my gitlab pipeline will fail (on a short-lived branch).

SonarQube analysis indicates that quality gate is failed.
Open Issues is failed: Actual value 37 > 0
Reopened Issues is passed: Actual value 0

SonarQube analysis reported 37 issues
2 critical
11 major
19 minor
5 info

SonarQube Developer Edition Version 7.1 (build 11001)
Branch Plugin 7 (build 11001)
Gitlab Plugin 3.0.1

Thanks


(Krzysztof Jazgara) #2

Hello,

The current branching implementation uses a hard-coded “mini” Quality Gate (No open issues) for short-lived branches. So this is currently fixed and cannot be changed.

So you are right on below:
“Do the quality gate profiles only apply to long-lived branches?”
Yes.

Greetings,
Kris


(Nicolas Bontoux) #3

To complement @Krzysztof_Jazgara’s answer, I recommend going through the Branch Analysis section of SonarQube documentation. More specifically to your queries: there’s a sub-page about short-lived branches which gives good insights on expected behaviour there.


(Bert) #4

Thank you for the clarification. The expectation for PR analysis is to consider the assigned quality profile for the project. Can you please give a hint when this will be supported.


(Nicolas Bontoux) #5

Hi Bert,

Careful with the wording here: Quality Profile used is the same, what you’re enquiring about is Quality Gate. There’s no immediate plan to enable full-blown custom Quality Gates on Pull Requests, we rather believe in making sure that no bug/vulnerability/code_smells slips through the cracks, allowing the dev to assess any issue (with the ability to only acknowledge if really needed) before merging.

Note that coverage support for PR is in the pipes:


(Bert) #6

Hi Nicolas,
sorry for the confusion. Of course I wanted to refer to the quality gate which is configured for the project.

The info about the support of the code coverage for PR is great :slight_smile:.
Will it also be possible to tweak the “hard coded” quality gate for PRs and filter issue severity e.g. ignore “Info” ?

Regards
Bert


(Nicolas Bontoux) #7

No plan to ignore upfront. As I said:

What I mean with that acknowledge is what’s mentioned in the Pull Request Analysis documentation:

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” all the issues available on the PR.

To be approached with care. This shouldn’t be used to cheat obviously, it’s only SonarQube’s way to let the ultimate decision in the dev’s hand, as there can be human/context-specific considerations that apply.


(Bert) #8

I understand and this workflow is great for green field projects. We use SonarQube many years and used the issue severity to highlight important issues. This is now not possible anymore with the hard-coded quality gate for PR analysis and we disabled the external approval of PR analysis. :frowning:

Updating the quality profiles for all languages and remove certain severities is not an option. Use a derived quality profile is also not possible because an activated rule cannot be disabled or removed.

Today I can only create copies of the quality profiles and remove the rules which shall not be applied for PR analysis. The developer shall not acknowledge issue which we will fix in another refactoring task (typically info or minor issues). I am not sure whether this the right way and it creates some effort as well.


(Joe Swanson) #9

It is wonderful that you believe that. I happen to believe that too. But the business has deadlines to meet that have marketing money on the line and coordinated product launches that sometimes cannot afford to be held up because someone commits code that has an informational issue raised in it. Or maybe even there isn’t a deadline but it is a prototype that is quickly evolving and quality isn’t the concern so much as proof of concept. Either way the organization should be able to make their own decisions about what quality is acceptable to them for all branches, short or long term. Enforcing a draconian zero leak policy that cannot be modified is ridiculous. The alternative is requiring an organization to increase their efforts maintaining their CI process with one off exceptions to quality gates or modifying rule sets rather than managing the actual quality gate at a global level to suit their ability to absorb technical debt while still meeting their organizational goals.

Assuming that a zero leak policy is even desirable, let alone feasible for everyone is short sighted and reduces the value of your own product in helping everyone improve their own products.

Personally we have had to turn off all quality gate checks because the zero leak policy became too detrimental to the business. It is horrible to not have an automated mechanism to hold developers accountable, but it is worse to hold them accountable to a level that loses money.


(Bert) #10

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.


(G Ann Campbell) #11

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


(Michael J Vinca) #12

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?


(G Ann Campbell) #13

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


(Bert) #14

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


(G Ann Campbell) #15

Hi Bert,

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

 
Ann


(Michael J Vinca) #16

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.


(G Ann Campbell) #17

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


(Chris Buchanan) #18

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.


(G Ann Campbell) #19

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 https://docs.sonarqube.org/latest/analysis/pull-request/

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


(Chris Buchanan) #20

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