nemccarthy / stash-pullrequest-builder-plugin

A Jenkins plugin for Building Stash Pull Requests
https://wiki.jenkins-ci.org/display/JENKINS/Stash+pullrequest+builder+plugin
Other
64 stars 130 forks source link

Allow deleting old build finish comments #62

Closed nickbroon closed 8 years ago

nickbroon commented 8 years ago

When Pull Request branches, or their target branches, are regularly updated, it can result in a lot of old Build Finish Comments on the PR, be they success/failure. As the only action on PR is to merge it, and this only applies to the most recent HEAD of the PR and the target branch, users/maintainers are generally only interested in the Build state of this, and not all the previous ones. So deleting the old ones, and thus only having the state of the most recent build cuts down on the noise in the PR comments. It make the PR Builder comment behave almost like a Build Status as found on Stash Commits. (aside: it would great if Stash supported Build Status for PR, not just commits. See https://jira.atlassian.com/browse/BSERV-8165). Controlled by a config option, that defaults to the existing behaviour. Tested, and works in my setup. Only the most recent Build Finish message is kept.

indrgun commented 8 years ago

I built the plugin from this branch of yours. However the old comments are not deleted.

In the Stashpullrequest log I configured i see:

Nov 23, 2015 3:33:47 PM FINE stashpullrequestbuilder.stashpullrequestbuilder.stash.StashApiClient Delete comment {https://.com/sjc/shared/1/rest/api/1.0/projects/CVGPI/repos/git-stash-training/pull-requests/36/comments/17745?version=0} returned result code; 204

nickbroon commented 8 years ago

Did you turn the option on in the advanced section? This is beyond my scope of knowledge of the plugin. I've not delved that far down into the REST API layer (yet), however the API Guide suggests 204 is the success code for deleting comments: https://developer.atlassian.com/static/rest/stash/3.11.3/stash-rest.html#idp916608 I'd guess that comments will need to have been created by the same plugin, using the same API key, user and permission. So for example any new comments created, will later be deleted if the PR (or it's target) have commits updated to it. I've not changed the methods that delete comments, just reuse them, so I'd expect you to see similar none deletion issues with for example the Build Start comments, as these are deleted when the Build Finish comments are added. Certainly for all new PR I've created my change does result in Finish comments being removed when the plugin rebuilds the PR and adds a new comment.

indrgun commented 8 years ago

Ok. Thanks. On Nov 23, 2015 4:33 PM, "Nick Brown" notifications@github.com wrote:

Did you turn the option in advance section? This beyond my scope of knowledge of the plugin. I've not delved that far down into the REST API layer, yet, however the API Guide suggest 204 is the success code for deleting comments: https://developer.atlassian.com/static/rest/stash/3.11.3/stash-rest.html#idp916608 I'd guess that comments will need to have been created by the same plugin, using the same API key, user and permission. So for example any new comments created, will later be deleted if the PR (or it's target) are have commits update to it. I've not changed the methods that delete comments, just reuse them, so I'd expect you to see similar none deletion issues with for example the Build Start comments, as these are deleted when the Build Finish comments are added. Certainly for all new PR I've created my change does result in Finish comments being removed when the plugin rebuilds the PR and adds a new comment.

— Reply to this email directly or view it on GitHub https://github.com/nemccarthy/stash-pullrequest-builder-plugin/pull/62#issuecomment-159113860 .

indrgun commented 8 years ago

Let me delete them all manually and retrigger new build. On Nov 23, 2015 4:41 PM, "Indra Gunawan" indrgun@gmail.com wrote:

Ok. Thanks. On Nov 23, 2015 4:33 PM, "Nick Brown" notifications@github.com wrote:

Did you turn the option in advance section? This beyond my scope of knowledge of the plugin. I've not delved that far down into the REST API layer, yet, however the API Guide suggest 204 is the success code for deleting comments: https://developer.atlassian.com/static/rest/stash/3.11.3/stash-rest.html#idp916608 I'd guess that comments will need to have been created by the same plugin, using the same API key, user and permission. So for example any new comments created, will later be deleted if the PR (or it's target) are have commits update to it. I've not changed the methods that delete comments, just reuse them, so I'd expect you to see similar none deletion issues with for example the Build Start comments, as these are deleted when the Build Finish comments are added. Certainly for all new PR I've created my change does result in Finish comments being removed when the plugin rebuilds the PR and adds a new comment.

— Reply to this email directly or view it on GitHub https://github.com/nemccarthy/stash-pullrequest-builder-plugin/pull/62#issuecomment-159113860 .

nickbroon commented 8 years ago

@nemccarthy should I submit this PR against https://github.com/jenkinsci/stash-pullrequest-builder-plugin?

indrgun commented 8 years ago

Thank you. On Nov 28, 2015 4:01 AM, "Nick Brown" notifications@github.com wrote:

@nemccarthy https://github.com/nemccarthy should I submit this PR against https://github.com/jenkinsci/stash-pullrequest-builder-plugin?

— Reply to this email directly or view it on GitHub https://github.com/nemccarthy/stash-pullrequest-builder-plugin/pull/62#issuecomment-160287682 .