Topic on Talk:Continuous integration/Codehealth Pipeline/Flow

Marking false positives

4
APatro (WMF) (talkcontribs)

Some of the issues raised by Sonarqube are false positives. Eg: https://sonarcloud.io/project/issues?id=mediawiki-extensions-Translate&issues=AWuKiMSZZf_vcVxnjMey&open=AWuKiMSZZf_vcVxnjMey - this property is being used during serialization.

As per this very recent blog post there are three ways of marking something as a false positive

I prefer the Mark as false positive approach as that would keep the code clean and we would not have dependency on the tool in our code. This approach would probably require us to be added as team members in the organization.

The second approach approach would be to use @SuppressWarnings(‘sf:UnusedPrivateField’), but we will have to add these throughout the code.

The //NOSONAR approach is obviously not recommended, as this would not display any other code smells or bugs either.

So my question is, is it possible to be added as team members for the organization, or would you recommend that we go with the second approach?


KHarlan (WMF) (talkcontribs)

I think the first approach (add you to the organization and give you permissions for extensions you maintain) is the best. There's a complication in that we might end up using two SonarQube instances – one for patches (SonarCloud.io) and a second self-hosted one for post-merge analysis, and the annoying thing is that someone would have to go through both instances to mark these as false positives. The advantage to the SuppressWarnings annotation is that you'd only have to do it once.

KHarlan (WMF) (talkcontribs)

As for the process of adding people to the organization – @GLederrey (WMF) do you have thoughts on how we should manage that?

GLederrey (WMF) (talkcontribs)

From experience, I have a preference for using @SuppressWarnings() annotation. It's a purely cognitive dependency (not a technical dependency, in the sense that the code will continue to work even if we don't use SonarQube). It allows to keep all sources of truth in the code itself, to tag false positive before even committing / pushing the code (provided you run analysis locally, which you probably should), and is more robust to refactoring.

Reply to "Marking false positives"