Azure DevOps PR decoration undocumented behavior?

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.

3 Likes

@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):

Hi,

I’m currently trying to get a reproducible scenario in order to troubleshoot this issue on my side to see what we can do to fix it.

Thanks !

Mickaël

Hi there,

As we did some tests we discovered a bug preventing some pr comment to not being deleted.

This fix has been deployed today.

Before going further with better experience on how SonarCloud should handle comments in Pull Request, could you please make some tests on your side to be sure that comments are, in the first place, being deleted in a consistent manner ?

Thank you in advance.

Mickaël

2 Likes

Hi,

Does anyone made further tests in its side and see any improvement on the comment deletion consistency ?

Thanks.

Mickaël

Hello @mickaelcaro,

I did some tests on a few or our PRs and SonarCloud is cleaning up it’s comments like we expect.
So, our issue is solved!

Thanks!
Freddy

1 Like

@mickaelcaro I also haven’t noticed anything not being cleaned up in the last week or so. So I think we have definitely seen an improvement.

Thanks,
Pete

Thank you !

Hey Team, since this issue is talking about PR decoration with comments being deleted. I wanted to add that this also effects monorepo types where one PR can have multiple sonar projects.

The current behavior wipes out all of the comments from SonarQube as a whole and not just the comments for the same SonarQube project.

Looks like this would resolve this undesired behavior.

Though it would be nice if we did leave the current/default “delete original comment” behavior, the decorator would only remove comments for the specific sonarQube project.

2 Likes

Did anyone ever figure out the first point listed in the original post:

“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?”

We have this exact same behavior and it’s very frustrating that it silently only adds 20 comments regardless of how many there actually are. Even if it only added 20 but then a “summary” comment that said the rest are viewable in SQ for example.

Yikes… 2 years later and found this as I was searching for “Why are all the sonarcloud comments deleted in my audit trail”… ugh.

Still not assigned? and likely not being fixed? I’m not sure what good it does to have sonarcloud auto-comment code, then remove the comment as though it never existed. Trying to explain to those that audit that “Well, there was a comment there, but we have no idea what it was” is silly.

Is there an update on the ability to turn on/off comment deletion?

1 Like

We just started using Sonarcloud and got surprised when we got the first comments on the PRs and they just got deleted like they wouldn’t have existed ever.

After reading this thread, I realize it’s apparently a feature at the moment. I really liked the suggestion and mockup by S.V.Groeneveld to let everyone choose how they want to handle the resolved ones. It’s a bit sad the task that was created out of this ([VSTS-184] Refine strategy about comments when an issue is resolved on SonarCloud - SonarSource) is still not 3 years later assigned or implemented. Is there any info whether it will be implemented or when we could possibly expect it?