Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[Notifier] Add notifier for Microsoft Teams#39007

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
Nyholm merged 1 commit intosymfony:5.xfromidetox:notifier-bridge-teams
Apr 8, 2021

Conversation

@idetox
Copy link
Contributor

@idetoxidetox commentedNov 5, 2020•
edited by OskarStark
Loading

QA
Branch?5.x
Bug fix?no
New feature?yes
LicenseMIT
Doc PRsymfony/symfony-docs#15193
Recipe PRsymfony/recipes#929

Add notifier bridge for Teams using Incoming Webhook implementation : seeIncoming Webhook using curl

EDIT by@OskarStark:
I removed the options logic in the first step and will create a follow up PR adding them back to the bridge ❗

simonberger, ismail1432, and OskarStark reacted with hooray emojiderrabus and OskarStark reacted with rocket emoji
@idetoxidetoxforce-pushed thenotifier-bridge-teams branch 2 times, most recently fromdd889cb to4a59f17CompareNovember 5, 2020 19:05
@jderussejderusse added this to the5.x milestoneNov 5, 2020
@idetoxidetoxforce-pushed thenotifier-bridge-teams branch 2 times, most recently from204679d to6764453CompareNovember 6, 2020 08:16
Copy link
Member

@fabpotfabpot left a comment

Choose a reason for hiding this comment

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

Can you add@experimental in 5.3 in all classes?

@OskarStark
Copy link
Contributor

What do you think of renaming the bridge toMicrosoftTeams orMsTeams?

nicolas-grekas, stloyd, and idetox reacted with thumbs up emoji

@OskarStark
Copy link
Contributor

Hi@idetox 👋

Are you interested in finishing this PR or am I allowed to take over?

Cheers Oskar

@idetox
Copy link
ContributorAuthor

Hey sry@OskarStark , been busy with the end of the year at work & family, should have more time soon (i should be able to do it nxt week), if the changes need to be done fast you can take over, let me know !

@OskarStark
Copy link
Contributor

OskarStark commentedJan 6, 2021•
edited
Loading

Hi 👋 thank you for the feedback, next week would be super 👌🏻👍🏻

@OskarStarkOskarStark changed the title[Notifier] Add notifier for Teams[Notifier] Add notifier for Microsoft TeamsJan 15, 2021
@OskarStark
Copy link
Contributor

As we are lacking tests for the many new classes I would like to introduce the bridge without custom options and rework the options part in another follow up PR.

In this case we have a bridge which can send chat messages to MicrosoftTeams, which is more than we are supporting right now.

WDYT@Nyholm@jderusse ?

Nyholm reacted with thumbs up emoji

@fabpot
Copy link
Member

Adding options in another PR works for me, that's what we did in the past for another provider (I don't remember which one though).

OskarStark reacted with thumbs up emoji

@OskarStarkOskarStarkforce-pushed thenotifier-bridge-teams branch 3 times, most recently from40b6563 to5ff55efCompareApril 7, 2021 07:19
@Nyholm
Copy link
Member

Dont forget the.gitignore =)

Wink wink#40700

OskarStark reacted with laugh emojiOskarStark reacted with rocket emoji

Copy link
Member

@NyholmNyholm 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 way more simple now.

Let me know when you are ready for a final review.

@OskarStark
Copy link
Contributor

Dont forget the .gitignore =)

Added

Copy link
Member

@NyholmNyholm left a comment•
edited
Loading

Choose a reason for hiding this comment

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

Thank you.
Im happy with this PR now.

Just make sure the CI is green.

@OskarStarkOskarStarkforce-pushed thenotifier-bridge-teams branch 3 times, most recently from5b2c154 to4e8bf02CompareApril 7, 2021 08:14
@OskarStark
Copy link
Contributor

All green now 👍

@Nyholm
Copy link
Member

Could you do a quick rebase before merge?

OskarStark reacted with thumbs up emoji

@Nyholm
Copy link
Member

Thank you Oskar and@idetox

OskarStark reacted with thumbs up emoji

@NyholmNyholm merged commit8bc0a8f intosymfony:5.xApr 8, 2021
OskarStark added a commit to symfony/symfony-docs that referenced this pull requestApr 8, 2021
This PR was merged into the 5.3-dev branch.Discussion----------[Notifier] Add MicrosoftTeamsDocs forsymfony/symfony#39007Best viewed with:https://github.com/symfony/symfony-docs/pull/15193/files?diff=unified&w=1Commits-------9e1e0a9 [Notifier] Add MicrosoftTeams
@fabpotfabpot mentioned this pull requestApr 18, 2021
fabpot added a commit that referenced this pull requestJun 23, 2021
…karStark)This PR was squashed before being merged into the 5.4 branch.Discussion----------[Notifier] Add options to Microsoft Teams notifier| Q             | A| ------------- | ---| Branch?       | 5.4| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Tickets       | Follows#39007| License       | MIT| Doc PR        |symfony/symfony-docs#15288### After rework:<img width="374" alt="CleanShot 2021-04-15 at 09 40 45@2x" src="https://user-images.githubusercontent.com/995707/114832421-c04c9400-9dce-11eb-8135-77ee1fb21314.png">Commits-------d039ce7 [Notifier] Add options to Microsoft Teams notifier
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

@OskarStarkOskarStarkOskarStark approved these changes

@NyholmNyholmNyholm approved these changes

@fabpotfabpotAwaiting requested review from fabpot

@stofstofAwaiting requested review from stof

@jderussejderusseAwaiting requested review from jderusse

@chalasrchalasrAwaiting requested review from chalasr

Assignees

No one assigned

Projects

None yet

Milestone

5.4

Development

Successfully merging this pull request may close these issues.

9 participants

@idetox@OskarStark@fabpot@Nyholm@nicolas-grekas@stof@jderusse@derrabus@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp