Skip to content

[Notifier] Add notifier for Clickatell #38922

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 25, 2021
Merged

[Notifier] Add notifier for Clickatell #38922

merged 1 commit into from
Jan 25, 2021

Conversation

ke20
Copy link

@ke20 ke20 commented Oct 31, 2020

Q A
Branch? 5.x
Bug fix? no
New feature? yes
Deprecations? no
License MIT

Add notifier bridge for Clickatell

@Nyholm
Copy link
Member

Nyholm commented Oct 31, 2020

Thank you for this PR.

Could you see why the fabbot.io is upset? They will give you some curl commands so it will fix things automatically.

@jderusse jderusse added this to the 5.x milestone Oct 31, 2020
@B-Galati
Copy link
Contributor

B-Galati commented Nov 2, 2020

A new recipe will be needed no?

ke20 pushed a commit to ke20/recipes that referenced this pull request Nov 2, 2020
Add recipe for the clickatell notifier - symfony/symfony#38922
da8b099
@ke20 ke20 mentioned this pull request Nov 2, 2020
Merged
@ke20
Copy link
Author

ke20 commented Nov 2, 2020

A new recipe will be needed no?

Yes thx, I created a PR for the recipe here : symfony/recipes#838

@ke20 ke20 requested a review from derrabus November 2, 2020 20:27
@jschaedl
Copy link
Contributor

jschaedl commented Nov 4, 2020

@ke20 As every bridge has its own LICENSE file could you please add one here as well?

@OskarStark
Copy link
Contributor

Typo in the PR title:
CleanShot 2020-11-20 at 12 16 34

@ke20 ke20 changed the title [Notifier] Add notifier for Clikatell [Notifier] Add notifier for Clickatell Nov 20, 2020
fabpot
fabpot requested changes Dec 5, 2020
View reviewed changes
Copy link
Member

@fabpot fabpot 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 a .gitignore file like done in other bridges?

OskarStark
OskarStark reviewed Jan 20, 2021
View reviewed 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.

Some small things, I think using null for from is more common

@wouterj wouterj removed their request for review January 20, 2021 10:35
@OskarStark
Copy link
Contributor

Hi @ke20 👋

Can you please rebase and apply my comments? Thanks

@ke20
Copy link
Author

ke20 commented Jan 22, 2021

Hi @ke20 wave

Can you please rebase and apply my comments? Thanks

Done @OskarStark

But Travis fails for PHP 7.4, there are flaky tests ?

I don't know if it's "normal".

OskarStark
OskarStark approved these changes Jan 22, 2021
View reviewed 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.

Test failures unrelated

@OskarStark OskarStark requested review from derrabus and fabpot January 22, 2021 10:10
@OskarStark OskarStark requested a review from jderusse January 22, 2021 10:10
@OskarStark
Copy link
Contributor

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

@OskarStark OskarStark merged commit ffc2c1e into symfony:5.x Jan 25, 2021
This was referenced Jan 25, 2021
Closed
Closed
@fabpot fabpot mentioned this pull request Apr 18, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas left review comments

@jschaedl jschaedl jschaedl left review comments

@OskarStark OskarStark OskarStark approved these changes

@xabbuh xabbuh xabbuh approved these changes

@chalasr chalasr Awaiting requested review from chalasr

@dunglas dunglas Awaiting requested review from dunglas

@lyrixx lyrixx Awaiting requested review from lyrixx

@sroze sroze Awaiting requested review from sroze

@yceruto yceruto Awaiting requested review from yceruto

@derrabus derrabus Awaiting requested review from derrabus

@fabpot fabpot Awaiting requested review from fabpot

@jderusse jderusse Awaiting requested review from jderusse

Assignees
No one assigned
Labels
Feature Notifier Status: Reviewed
Projects
None yet
Milestone
5.4
Development

Successfully merging this pull request may close these issues.