Over the past few weeks of using Sonar, it has come to my attention that older PRs that have not been rebased pick up changes that are unrelated to the pull request themselves. I think this is because the diff happens directly against the HEAD instead of against the merge-base. Can we have this change so as to show what is relevant to the code irrespective of a rebase.
I assure you this is not the case. Our pull request analysis has always used the merge-base to compute the changed lines. That is, in a pull request analysis, we take the HEAD of the working tree (the tip of the pull request’s branch), then look for the common ancestor with the specified base branch (that the pull request wants to be merged into, set with sonar.pullrequest.base or auto-detected depending on your setup). From this we have the range of commits in the pull request that are not present in the base branch, and from this diff we compute the lines that have changed.
When you browse the code of the pull request, the changed lines are highlighted with yellowish background. These highlighted lines should correspond to what you would see with the command:
git diff the-base-branch...HEAD
A common issue is when we cannot find the common ancestor, for example when the CI uses a shallow clone that doesn’t have enough commit history to include the common ancestor. In this case you would see a warning on SonarCloud, telling you about it.
I hope this explanation helps. If you still think there is an issue, then we will need more details about your setup to understand what’s happening, such as the analysis parameters and the CI system you’re using. The scanner logs may also be useful, and if there are any warnings visible on the pull request’s page on SonarCloud UI.
2 Likes
You were absolutely right, the issue was with us using a ‘shallow’ checkout to save on time. I would recommend however that mentioning that this would break diffs on your error messages could prove useful. We didn’t think much of it as it said it prevents features like auto-assigning of issues from working, which weren’t being used to begin with, that along with disabling honorRefspec gave us a false sense that things should work on diff land.
1 Like
This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.