search

AmadeusITGroup / sonar-stash

Stash (BitBucket) plugin, a pull-request decorator which allows to integrate SonarQube violations directly into your pull-request
MIT License
165 stars 82 forks source link

sonar-stash should ignore sonar issues not belong to diff view #169

Closed kingoleg closed 6 years ago

kingoleg commented 6 years ago

Hi,

This is something I patched in my fork, but now Amadeus master and 1.3 are far away of that and it is not possible to create a PR.

So, the issue is that it is possible that sonar-stash analysis will found a new issue while PR diff doesn't consist the change. It is base of workflow we use.

  1. We have a master branch. Full sonar scan is doing by schedule once a day
  2. PR sonar scan is doing per request, when PR is created, for example.
  3. Before PR sonar scan, we merge master into feature branch
  4. If there are several PR created and merged into master before the latest feature branch PR sonar scan, sonar will found issues from other branches as new

What is wanted:

Per each issue sonar-stash found, we have to filter it thru PR diff before use it in PR overview comment or in decision to approve PR or not

Regards, Oleg

kingoleg commented 6 years ago

169 , #165 , #114, #135, #58 are all about the same

t-8ch commented 6 years ago

Hi @kingoleg,

Unfortunately it is not that easy. We can't simply assume that inside diff view == true positive and outside diff view == false negative. The only way to have this correct is by performing a scan on the correct PR baseline. (And doing a rescan if the baseline changes). Anything else will result in wrong behaviour.

For example: You modify your tests so that less code is covered. The issue will be reported to the file where the code is defined, not where you modified the tests. If we ignore this issue people will be unhappy.

kingoleg commented 6 years ago

Unfortunately, there is no option to rescan baseline after each PR merge. It takes ~1 hour.

In our case it's better to have quite good PR check than "ideal" PR check with a lot of garbage belong to code that is not exist in diff/PR