Azure DevOps PR decoration undocumented behavior?

We’re using SonarCloud integration with Azure DevOps, and have enabled PR decoration.
This all works fine, we can see comments being generated in Pull Requests.

But we’re noticing a few oddities:

  • On a large pull request there were 20 comments generated by SonarCloud, but when we navigated to the SonarCloud PR analysis page there were actually 75 issues found. And every PR build it would add a few new comments to the PR… Is there some undocumented max limit on the number of comments it can post on a PR?

  • When we change the issue status within SonarCloud, it automatically updates the status accordingly in the Pull Request. But when we set the status in the PR to “WontFix” upon next build it will mark the issue in SonarCloud as “Fixed” (instead of “WontFix”).

  • SonarCloud comments seem to be deleted from PR’s after resolving them. We never delete our manual comments, since this is making it difficult to read historical PR’s and understand the reason things got changed. Why is SonarCloud doing this?

I’m not aware of such limit. Maybe @Julien_HENRY can give more insights.

When you say “when we set the status in the PR to WontFix”, you mean when you do it in Azure DevOps UI?

I guess the idea is to not “pollute” too much the PR with useless comments (when issues are fixed, there’s no more problem). You would expect the comment to be set to “Closed”?

We have also encountered the 20 comment limit cap on VSTS, we had just assumed it was some sort of limitation.

Sorry, that was indeed unclear. All status changes that are done in SonarCloud get reflected immediately in the Azure DevOps PR, that works fine.
But when we change the status in the Azure DevOps PR to “WontFix”, the status on SonarCloud will be “Fixed” after next build.

Exactly, either marked as closed, resolved or wontfix… which are the three “inactive” states in Azure DevOps. But it’s the author of the PR that should decide on that status…
We have people actually replying to SonarCloud’s comments how they fixed it, or how they defend why they won’t fix it. If SonarCloud then deletes its own comment, you end up with a bunch of replies to a “* Comment deleted
I don’t think any PR system hard-deletes comments when issues are resolved, but most implement some way to automatically hide resolved or closed comments.

Thanks for the details. I tried to reproduce this behaviour, and I could not. There must be something missing in the scenario. Are you sure that some part of the code (even not on the very same line) did not get changed and therefore resolved the issue?

Indeed, when relying on a comment-based feature to show automatic review issues, maybe we should behave that way. I’m creating VSTS-184 to track this.

Thanks!

Has the non-delete feature been implemented now? Because we are now seeing all of our SonarCloud comments are not being deleted and always remain as ‘active’ once they have been created.

As you can see on VSTS-184, nothing has been done on this topic for now.

Can you clarify what changed on your end? I’ve looked at some projects on my side, and everything seems to work as expected.

Hi @Fabrice_Bellingard,

The comment feature works as expected, but once we solve the issue the comment is no longer deleted by the SonarCloud user, whereas it always used to be.

The screenshot shows the commented code has been deleted, but the SonarCloud comment still remains.

Thanks,
Pete

Hi @pconnor!

Following your message, I’ve done some tests to check if something had changed in the behaviour, and on my side, everything works as expected: the comments get deleted/removed when I fix issues inside the branch of a given PR. Here’s what I did for my tests:

  • I create a branch
  • I update one file to add an obvious issue (like adding a public field to a class)
  • I create a PR for that branch
  • Once the analysis is done, I check that the issue (comment) is available on the PR
  • Then I fix the issue on the branch and push the change
  • Once the analysis is done, I check that the issue (comment) is no longer visible

I did a couple of variations (like answering the issue comment before fixing it), and every time it works as expected (= the issue comment disappears).

I suspect there is something specific in your use case that we’re missing, but I don’t see what.

The main question is: ‘what is expected?’…

I noticed yesterday that a SonarCloud comment on a fixed issue was not deleted upon next analysis during PR build. And I’m totally happy with that!

We always have developers reply to a comment and mark it as ‘Resolved’ in Azure DevOps right after they push their fix. Would that manual status change have any influence on SC behavior?

The currently expected behaviour is: the issue comments are no longer visible from the PR when an issue gets fixed in a PR.

I’d like to get @pconnor’s perspective on this. It seems to me that you might not expect the same behaviour. Should issue comment be removed or not when an issue gets fixed?

Thanks for this info! This might help reproduce this “unexpected” behaviour.

We have just recently obtained ISO 27001 accreditation, so anything that can be used as an audit trail / show we are sticking to procedures is a good thing. I guess in an ideal world for us would be when an issue raised by SonarCloud has been fixed, the original comment would be marked as fixed and there would be an additional comment from SonarCloud under the original one saying “This is has now been resolved”.

The only reason I questioned this change for us originally is our devs were starting to think that they hadn’t fixed their issues because the comments were still remaining, it was just different to the behaviour they were used to. But if we can show in some form or another than the issue has been fixed (status / additional comment) then that would be ideal.

Thanks for your insightful feedback @pconnor!

Hey guys,

I got pointed at this discussion by Peter in a different thread about SC not cleaning up its comments.

We noticed this also for a while now and I agree with Peter his last comment. Currently SC places a comment for an issue, but does not delete/resolve it when it is fixed. This is confusing for some people.

Personally I would prefer removal of the comment, since this keeps the PR thread nice and tidy.
But let SC resolving/closing the original comment with a new comment on it would also help us.

Best regards,
Freddy

(my thread: SonarCloud does not cleanup it's comments anymore on a PR in Azure DevOps )

It’s funny how we already have three very different expectations of what SonarCloud is “supposed to do” with its own comments in pull requests upon detecting that they have been resolved.

To me it feels that SonarCloud’s main feature is to signal potential issues and post them as a comment… How these comments are resolved afterward, is a company-specific or team-specific process or preference.

I could see a lot of value in an additional configuration option in SonarCloud that controls this ‘cleanup’ behavior. Refer to my mockup below, of course this could be lifted to higher level if it would also apply to other providers.

Just to be clear: we would choose “No action” since we want to be able to trace back what happened in a pull request (issues found, and whether/how they got resolved), especially since Azure DevOps supports comment statuses like “Resolved” or “WontFix”, and already automatically hides comments that have been marked as such.

@S.V.Groeneveld That is a really good idea and would cater to everyone’s needs!

For the record, I’m tracking this topic on VSTS-184. Your suggestion @S.V.Groeneveld is a good basis to discuss further.

Now, I’d really like to understand why SonarCloud currently does not handle comments in a consistent manner. @mickaelcaro, could you drive the investigation please?

1 Like

Not sure if things changed on SonarCloud end, but the comment deletion behavior seems to be acting stranger lately.

We are seeing SonarCloud-generated comments being deleted while they are still applicable (nothing had changed in the related code), resulting in orphaned comment threads. The affected issues are still “Open” on SonarCloud side itself.
And at the same time, in the same PR build, it leaves comments in place about issues that have been fixed.

Something going wrong with identifying the right comments?

Can’t easily share an example where a comment is left in place for a resolved issue, but here’s one case of a (deleted) comment for an unresolved issue(Web:BoldAndItalicsTagsCheck):
image

Unrelated, but here’s an example of a (deleted) comment for a resolved issue (javascript:S4144):