Skip to content
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

Adding a trait to allow disabling build notifications. #132

Closed
wants to merge 1 commit into from

Conversation

alex-dubrouski
Copy link

We rely heavily on BitBucket Branch Source plugin, but for couple of root builds we need to disable notifications. These builds are verifying custom scenarios and failure of a build is not an unexpected scenario, though it results in multiple build statuses being posted back to repository which is confusing and annoying. I noticed that there is already parameter which disables build notifications and decided to add a trait to expose it.

@jetersen
Copy link
Member

Since notifications is core to this plugin one could argue this trait should stay in this plugin. cc @stephenc 😅

Copy link
Member

@jetersen jetersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@stephenc
Copy link
Member

I think this should belong in an extension plugin. I believe that people should be able to disable (which is why it is exposed at the API level) but that the majority of users should not be exposed to the option until they need it... hence extension plugin

Closing because we will not merge this change in core. The code itself is good and looks correct, but belongs in an extension plugin. @Casz might help show you examples of how to do that

@stephenc stephenc closed this Jul 17, 2018
@jetersen
Copy link
Member

@alex-dubrouski see #127 (comment) for reference 😅

@alex-dubrouski
Copy link
Author

Thanks anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants