search

AmadeusITGroup / sonar-stash

Stash (BitBucket) plugin, a pull-request decorator which allows to integrate SonarQube violations directly into your pull-request
MIT License
165 stars 82 forks source link

Approve PR only when all issues are below threshold severity #140

Closed kzaikin closed 7 years ago

kzaikin commented 7 years ago

This will fix #95 These changes will modify default behavor which should be discussed.

If there are consumers who want PR to be approved when there are zero issues, then additional threshold value should be added. Something like sonar.stash.reviewer.approval.issue.severity.threshold However, this can be leveraged by lowing down sonar.stash.task.issue.severity.threshold

My opinion is to keep number of settings as low as possible and NOT to add any additional thresholds. Approving a PR as soon as there are no issue-tasks left looks quite natural so I'd stay with that as default behavior.

t-8ch commented 7 years ago

Hi @kzaikin, thanks for this!

FYI #95 is for a different usecase. In the SonarQube WebUI you can define lots of conditions when to accept the state of a project. Unfortunately this is not yet evaluated for preview builds.

To be honest I would prefer to have the preview functionality from SonarQube side than to implement all kinds of conditions in the plugin.

Do you have an urgent to use this new setting on your end or would you be fine with waiting for SonarSource to implement this on their side?

kzaikin commented 7 years ago

@t-8ch I have a dozen teams inside my company waiting for this change I think we can modify the code to use Sonar's quality gates later, as soon as quality gates are available in preview analysis

t-8ch commented 7 years ago

@kzaikin Fair enough. I would prefer a dedicated setting though. (In the hope of being able to delete it soon...) It would also be nicer to have a comparator implementation for severities, so we do not have to pass around lists of strings...

t-8ch commented 7 years ago

@kzaikin I implemented a version of this in #141. Would this cover your usecase?

t-8ch commented 7 years ago

Superseeded by #141