-
-
Notifications
You must be signed in to change notification settings - Fork 520
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#790: Add an ability to edit Summary Note instead of creating a new one #791
Conversation
We used to edit summary notes but moved away from it for a few reasons:
I'm presuming the driver behind this is that Gitlab doesn't collapse summary notes when the thread is resolved, so would suggest an alternative approach of creating a thread to add summary comments to with the initial message being |
Sorry for the late response.
Yes, it's true. But Gitlab notifies if all issues have been resolved. Expect another situation. Developer opens MR, SonarQube summary note is OK. Then developer pushes new commits, and every time SonarQube is OK. It sends a lot of annoying notifications.
I don't know how exactly SonarQube Developer Edition decorates MR's. All I have found is this video. As I understand it updates existing summary note, and does not creates a new one. Other tools also usually edit summary notes in Gitlab's MR. Gitlab's junit reports, code climate reports in MR also shows only the current state. Also in our experience conversations usually spawns on individual line comments and never on summary note. I presume that somebody should have a history for any change, so I added the configuration option to save the same behavior.
As I understand from the mentioned video, current behavior is already non-standard. Many forks from this repository implements similar options, but that forks are not very popular or are not supported in long term. I suppose that making configuration options for some different behaviors would satisfy community needs. Some additional documentation can be added to
Not only collapsing, also annoying notifications. I suppose that in your approach viewing the latest (actual) summary would not be convenient. |
The linked video doesn't show a re-scan of a Merge Request, only the manual change of the status of a finding and a (realtime?) submission of a change in status to Gitlab. The video appears to show that the existing comment is deleted and a new one submitted given the lack of I also don't want us adding more configuration flags into the plugin unless there's a strict requirement to need a flag (i.e. the plugin won't work without setting a certain value in a configuration flag). The plugin takes an opinionated approach to operation to save on the overhead of testing combinations of configuration or handing different set-ups when investigating issues. |
I really want to this feature as well. currenlty a lot of the PRs look like the screenshot below in GitHub for us. It would be great if we could reduce the noise by updating an existing comment. |
I've been pushing sonarqube in our CI and this has been one of the reviews of our users as well. It creates too much noise since it notifies everyone for every commit. In github actions, if we want to get notified of failures then we only need to add the Quality Gate check action. Ideally we only want to get emails if it's a user review, not for PR decorations. But I do agree that we should caution against introducing new flags. So either we implement this, or not. |
Closes #790