-
Notifications
You must be signed in to change notification settings - Fork 149
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
feat: allow granular selection of GitHub notification method via destination #304
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Tony Li <[email protected]>
Signed-off-by: Tony Li <[email protected]>
fb88628
to
a7a1cee
Compare
pkg/services/github.go
Outdated
sendComment bool | ||
) | ||
|
||
switch dest.Recipient { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i would suggest instead create variables, split them into different functions and execute inside switch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated. Couldn't use switch without making it very complex.
Signed-off-by: Tony Li <[email protected]>
Signed-off-by: Tony Li <[email protected]>
d34faba
to
56a3811
Compare
Ping @pasha-codefresh or any other owner? |
Will review. Sorry for delay |
Right now if someone has all of the GH notification types configured, subscribing to it will update all of them without a way to choose which one.
This is useful when you want to use commit status for one type of trigger, and deployments for another.
The change retains the existing behavior: if no recipient is specified in the subscription annotation, all notification types will be updated.