Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Notifier] Add TurboSms Bridge#41522
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
b936c71 to486ac94Comparefre5h commentedJun 3, 2021
Travis build failed because of I added a new package to the composer.jsonhttps://github.com/symfony/symfony/pull/41522/files#diff-08024b4d552345610ae41302c94fd454bae1875e86c8eb751409172aa249514cR84 But this package doesn't exist yet. |
nicolas-grekas left a comment
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.
thanks for submitting
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Notifier/Bridge/TurboSms/Tests/TurboSmsTransportFactoryTest.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
chalasr commentedJun 3, 2021
I'm not super comfortable with adding a bridge to a service whose documentation is not available in english, that limits the potential adoption quite a lot. (note that we don't have any policy yet, I'm just wondering). |
OskarStark left a comment
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.
@chalasr valid point but IMHO there should be no need for the docs unless it works like expected? WDYT? The nice thing is that I can use this provider while I cannot if there is no bridge available.
src/Symfony/Component/Notifier/Bridge/TurboSms/TurboSmsTransportFactory.phpShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Notifier/Bridge/TurboSms/TurboSmsTransport.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
chalasr commentedJun 3, 2021
If one day the implementation becomes outdated or no longer works as expected for whatever reason, who will be able to fix it? |
OskarStark commentedJun 3, 2021
IIRC we already have some bridges which have the same problem. This should not be an argument, just an info. |
StaffNowa commentedJun 4, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@chalasr our community a big :) I am provided two companies from Lithuania and if needed to help read docs it is not a big issue :))) one company officially have English translation, another company have only LT :) |
StaffNowa commentedJun 4, 2021
As I see TurboSMS is a UA company, but page loads Russian :) Too know Russian no problem 😊😁 |
fabpot commentedJun 4, 2021
My 2cts: Providers (notifier, mailer, translation, ...) MUST be maintained by the community (the developers using them), the core team cannot maintain them except for mass changes (CS, new PHP version, ...). |
StaffNowa commentedJun 4, 2021
Agree with@fabpot 👍 |
chalasr commentedJun 4, 2021
Works for me. |
src/Symfony/Component/Notifier/Bridge/TurboSms/TurboSmsTransport.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Notifier/Bridge/TurboSms/Tests/TurboSmsTransportFactoryTest.phpShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
StaffNowa commentedJun 6, 2021
You will need to make fabbot happy (rebase) :) |
9a25794 to19ee1a4Comparefre5h commentedJun 7, 2021
rebased |
OskarStark commentedJul 13, 2021
Can you please rebase? Thanks |
19ee1a4 tof2d965eComparefre5h commentedAug 2, 2021
@OskarStark rebased |
a0ce72c to12bacf4CompareOskarStark commentedAug 4, 2021
Thank you Artem. |
This PR was merged into the 5.4 branch.Discussion----------[Notifier] Add TurboSms BridgeTurboSms Bridge:symfony/symfony#41522Commits-------10cced4 Add TurboSms notifier
Uh oh!
There was an error while loading.Please reload this page.
Add TurboSms bridge to Symfony Notifier
https://turbosms.ua/api.html This service is very popular in Ukraine