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

Add Microsoft Teams notification #153

Merged

Conversation

KeisukeYamashita
Copy link
Member

What

I added microsoftteams field for Microsoft Teams notification configuration which was added in the Spinnaker 1.23.0 release.

The fields are same as the halconfig described here.
Thanks in advance.

Why

It was missing.
Described in Microsoft Teams vs Slack: Which Collaboration App Is Better?, there are many Microsoft Teams adoption in the world. We should add them soon.

Ref

Comment on lines +4954 to +4968















Copy link
Member Author

@KeisukeYamashita KeisukeYamashita Nov 15, 2020

Choose a reason for hiding this comment

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

[note]
Theses are auto generated by the kleat-protobuilder. I ran make proto to generate these fields.

Copy link
Contributor

@ezimanyi ezimanyi left a comment

Choose a reason for hiding this comment

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

This looks great, thanks for adding this!

@ezimanyi ezimanyi added the ready to merge Reviewed and ready for merge label Nov 16, 2020
@mergify mergify bot merged commit 16d6813 into spinnaker:master Nov 16, 2020
@mergify mergify bot added the auto merged label Nov 16, 2020
@maggieneterval
Copy link
Contributor

@KeisukeYamashita I think we need to also update the echo message and translate the microsoftteams block from the halconfig to the echo config, right?

@KeisukeYamashita
Copy link
Member Author

@maggieneterval Thanks for your quick review🙇‍♂️ Yeah, I totally missed that point, I will raise another PR🙇‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto merged ready to merge Reviewed and ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants