-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Notifier] Add bridge for smsc.ru #42180
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
Conversation
Great. Thank you for this PR. It looks like a few CI jobs are complaining. Could you make sure that at least "fabbot.io" and "Package / Verify" are green? |
8901968
to
a7cfda2
Compare
Sure, fixed all the CI issues I could. There still is a failed test, but it seems to be unrelated. |
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.
Cool cool cool.
Just a few minor things an I'll be happy.
--
Correct. That test failure is unrelated. Thank you for checking.
2d53fe7
to
0863f6d
Compare
6b70d93
to
0a69dc2
Compare
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.
Thank you.
Im happy with this PR. The psalm issue is addressed in #42183.
I think @OskarStark will be happy to review this PR later.
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.
Just some minor comments.
- Can you please rebase to get rid of the Psalm errors
- Please create a recipe at
symfony/recipes-contrib
repo, thanks
src/Symfony/Component/Notifier/Bridge/Smsc/Tests/SmscTransportFactoryTest.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Notifier/Bridge/Smsc/SmscTransportFactory.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Notifier/Bridge/Smsc/SmscTransport.php~Stashed changes
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Notifier/Bridge/Smsc/SmscTransportFactory.php~Stashed changes
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Notifier/Bridge/Smsc/Tests/SmscTransportFactoryTest.php~Stashed changes
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Notifier/Bridge/Smsc/Tests/SmscTransportTest.php~Stashed changes
Outdated
Show resolved
Hide resolved
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.
Please create a recipe for this bridge
Thanks Valentin for working on this feature, this is much appreciated. |
This PR was merged into the 5.4 branch. Discussion ---------- [Notifier] Add bridge for smsc.ru Add Notifier brdige for smsc.ru symfony/symfony#42180 Commits ------- 5f443b9 [Notifier] Add bridge for smsc.ru
Thanks for the fast and detailed review, guys Here's the recipe PR: symfony/recipes#975 |
Hi,
This is a notifier bridge for Russian SMS provider smsc.
This PR contains the new bridge and updates to framework bundle in all the places where all the other notifiers are listed.