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 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

Merged
OskarStark merged 1 commit intosymfony:5.4fromfre5h:feature-notifier-turbosms
Aug 4, 2021

Conversation

@fre5h
Copy link
Contributor

@fre5hfre5h commentedJun 2, 2021
edited
Loading

QA
Branch?5.4
Bug fix?no
New feature?yes
Deprecations?no
LicenseMIT
Doc PRsymfony/symfony-docs#15403
Recipe PRsymfony/recipes#953

Add TurboSms bridge to Symfony Notifier

https://turbosms.ua/api.html This service is very popular in Ukraine

@fre5h
Copy link
ContributorAuthor

Travis build failed because of

  Problem 1    - Root composer.json requires symfony/turbo-sms-notifier, it could not be found in any version, there may be a typo in the package name.

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.
I don't now how to fix CI correctly. Should I remove this package from composer.json? Or is there some other trick?

@nicolas-grekasnicolas-grekas added this to the5.4 milestoneJun 3, 2021
Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

thanks for submitting

@chalasr
Copy link
Member

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).

Copy link
Contributor

@OskarStarkOskarStark left a 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.

@chalasr
Copy link
Member

valid point but IMHO there should be no need for the docs unless it works like expected? WDYT?

If one day the implementation becomes outdated or no longer works as expected for whatever reason, who will be able to fix it?
Not providing an internationalized entry point for a product is either a sign of a lack of resources or it means the product targets a very specific, localized population.
Maybe this should just be released as a community bundle until the service has a universally accessible entry point?
I won't vote against this PR, but I don't feel safe regarding our ability to maintain this transport on the long run.

@OskarStark
Copy link
Contributor

IIRC we already have some bridges which have the same problem. This should not be an argument, just an info.

@StaffNowa
Copy link
Contributor

StaffNowa commentedJun 4, 2021
edited
Loading

@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
Copy link
Contributor

As I see TurboSMS is a UA company, but page loads Russian :) Too know Russian no problem 😊😁

chalasr reacted with thumbs up emoji

@fabpot
Copy link
Member

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 and vuphuong87 reacted with thumbs up emoji

@StaffNowa
Copy link
Contributor

Agree with@fabpot 👍

@chalasr
Copy link
Member

Works for me.

@StaffNowa
Copy link
Contributor

You will need to make fabbot happy (rebase) :)

OskarStark reacted with thumbs up emoji

@fre5hfre5hforce-pushed thefeature-notifier-turbosms branch from9a25794 to19ee1a4CompareJune 7, 2021 06:30
@fre5h
Copy link
ContributorAuthor

rebased

StaffNowa, OskarStark, and wbrframe reacted with thumbs up emoji

@OskarStark
Copy link
Contributor

Can you please rebase? Thanks

@fre5hfre5hforce-pushed thefeature-notifier-turbosms branch from19ee1a4 tof2d965eCompareAugust 2, 2021 06:57
@fre5h
Copy link
ContributorAuthor

@OskarStark rebased

@OskarStarkOskarStark self-requested a reviewAugust 2, 2021 12:45
@OskarStarkOskarStarkforce-pushed thefeature-notifier-turbosms branch froma0ce72c to12bacf4CompareAugust 4, 2021 14:40
@OskarStark
Copy link
Contributor

Thank you Artem.

@OskarStarkOskarStark merged commit52ce2af intosymfony:5.4Aug 4, 2021
OskarStark added a commit to symfony/symfony-docs that referenced this pull requestAug 4, 2021
This PR was merged into the 5.4 branch.Discussion----------[Notifier] Add TurboSms BridgeTurboSms Bridge:symfony/symfony#41522Commits-------10cced4 Add TurboSms notifier
@fre5hfre5h deleted the feature-notifier-turbosms branchAugust 5, 2021 06:22
This was referencedNov 5, 2021
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

@derrabusderrabusAwaiting requested review from derrabus

@NyholmNyholmAwaiting requested review from Nyholm

Assignees

No one assigned

Projects

None yet

Milestone

5.4

Development

Successfully merging this pull request may close these issues.

7 participants

@fre5h@chalasr@OskarStark@StaffNowa@fabpot@nicolas-grekas@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp