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

[Notifier] Add Gitter Bridge #39838

Merged

Conversation

christingruber
Copy link
Contributor

@christingruber christingruber commented Jan 14, 2021

Q A
Branch? 5.x
Bug fix? no
New feature? yes
Deprecations? no
License MIT
Doc PR symfony/symfony-docs#14835

Adds a notifier bridge for https://gitter.im the API documentation https://developer.gitter.im/docs/rest-api.

@carsonbot
Copy link

Hey!

Excellent, just like I would have done it.

To help keep things organized, we don't allow "Draft" pull requests. Could you please click the "ready for review" button or close this PR and open a new one when you are done?

Note that a pull request does not have to be "perfect" or "ready for merge" when you first open it. We just want it to be ready for a first review.

Cheers!

Carsonbot

@christingruber christingruber marked this pull request as ready for review January 14, 2021 15:54
@christingruber christingruber force-pushed the feature/notifier-gitter-bridge branch 2 times, most recently from 029436a to 0fb67c8 Compare January 14, 2021 17:05
@christingruber christingruber changed the title [Notifier] Add gitter bridge [Notifier] Add Gitter Bridge Jan 14, 2021
Copy link
Contributor

@OskarStark OskarStark left a comment

Choose a reason for hiding this comment

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

Thank you for this great PR @christingruber 👍

Just some comments

@christingruber christingruber force-pushed the feature/notifier-gitter-bridge branch 3 times, most recently from 18d2f4c to c074462 Compare January 14, 2021 22:46
@nicolas-grekas nicolas-grekas added this to the 5.x milestone Jan 15, 2021
Copy link
Contributor

@OskarStark OskarStark left a comment

Choose a reason for hiding this comment

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

Some changes

Copy link
Contributor

@OskarStark OskarStark left a comment

Choose a reason for hiding this comment

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

Thank you, please make fabbot happy

@OskarStark
Copy link
Contributor

Could you please create a docs PR and a recipe? Thanks

@Nyholm
Copy link
Member

Nyholm commented Jan 20, 2021

I am not super confident with this component. But I think it looks alright.

👍

It needs a rebase before we can merge it.

@christingruber
Copy link
Contributor Author

Rebase is done, thank you 👍

@OskarStark
Copy link
Contributor

Thanks Christin for working on this feature, this is much appreciated.

@OskarStark OskarStark merged commit 4818b28 into symfony:5.x Jan 20, 2021
OskarStark added a commit to symfony/symfony-docs that referenced this pull request Jan 21, 2021
…ruber)

This PR was merged into the 5.3-dev branch.

Discussion
----------

[Notifier] Add symfony/gitter-notifier docs

See symfony/symfony#39838

Commits
-------

26e88f7 Add symfony/gitter-notifier docs
@fabpot fabpot mentioned this pull request Apr 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants