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.

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

[Notifier] [Bridge] [KazInfoTeh] added the bridge #44360

Merged
merged 1 commit into from Jan 19, 2022
Merged

[Notifier] [Bridge] [KazInfoTeh] added the bridge #44360

merged 1 commit into from Jan 19, 2022

Conversation

taranovegor
Copy link
Contributor

@taranovegor taranovegor commented Nov 30, 2021 •
edited

Q A
Branch? 6.1
Bug fix? no
New feature? yes
Deprecations? no
Tickets -
License MIT
Doc PR symfony/symfony-docs#16088
Recipe symfony/recipes#1031

This is a notifier bridge for Kazakh SMS provider KazInfoTeh.

@carsonbot carsonbot added this to the 5.4 milestone Nov 30, 2021
@taranovegor taranovegor mentioned this pull request Nov 30, 2021
Merged
@xabbuh xabbuh modified the milestones: 5.4, 6.1 Dec 1, 2021
@OskarStark
Copy link
Contributor

Please also add a recipe like done for the other notifier bridges, thanks

@taranovegor taranovegor requested a review from stof December 5, 2021 16:41
OskarStark
OskarStark reviewed Dec 5, 2021
View reviewed changes
src/Symfony/Component/Notifier/Bridge/KazInfoTeh/KazInfoTehTransport.php Outdated Show resolved Hide resolved
src/Symfony/Component/Notifier/Bridge/KazInfoTeh/KazInfoTehTransport.php Outdated Show resolved Hide resolved
src/Symfony/Component/Notifier/Bridge/KazInfoTeh/KazInfoTehTransport.php Outdated Show resolved Hide resolved
src/Symfony/Component/Notifier/Bridge/KazInfoTeh/README.md Outdated Show resolved Hide resolved
src/Symfony/Component/Notifier/Bridge/KazInfoTeh/README.md Outdated Show resolved Hide resolved
src/Symfony/Component/Notifier/Bridge/KazInfoTeh/README.md Outdated Show resolved Hide resolved
src/Symfony/Component/Notifier/Bridge/KazInfoTeh/README.md Outdated Show resolved Hide resolved
src/Symfony/Component/Notifier/Bridge/KazInfoTeh/README.md Outdated Show resolved Hide resolved
src/Symfony/Component/Notifier/Bridge/KazInfoTeh/composer.json Outdated Show resolved Hide resolved
src/Symfony/Component/Notifier/Bridge/KazInfoTeh/composer.json Outdated Show resolved Hide resolved
OskarStark
OskarStark approved these changes Dec 6, 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.

Thanks

welcoMattic
welcoMattic approved these changes Dec 6, 2021
View reviewed changes
Copy link
Member

@welcoMattic welcoMattic left a comment

Choose a reason for hiding this comment

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

Thanks!
(We can ignore fabbot error)

fabpot
fabpot requested changes Jan 17, 2022
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.

Looks like you've missed the needed changes in src/Symfony/Component/Notifier/Exception/UnsupportedSchemeException.php and src/Symfony/Component/Notifier/Transport.php.

src/Symfony/Component/Notifier/Bridge/KazInfoTeh/composer.json Outdated Show resolved Hide resolved
src/Symfony/Component/Notifier/Bridge/KazInfoTeh/LICENSE Outdated Show resolved Hide resolved
fabpot
fabpot reviewed Jan 18, 2022
View reviewed changes
src/Symfony/Component/Notifier/Bridge/KazInfoTeh/KazInfoTehTransportFactory.php Outdated
*/
protected function getSupportedSchemes(): array
{
return ['kazinfoteh'];
Copy link
Member

Choose a reason for hiding this comment

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

As you've kept KazInfoTeh, I would use kaz-info-teh as the scheme here for consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello

Fixed in #44360, symfony/symfony-docs#16088 and symfony/recipes#1031.

Should we also replace KAZINFOTEH_DSN replace with KAZ_INFO_TEH_DSN?

Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also replace KAZINFOTEH_DSN replace with KAZ_INFO_TEH_DSN?

Yes please

@fabpot
Copy link
Member

fabpot commented Jan 19, 2022

Thank you @taranovegor.

@fabpot fabpot merged commit 0a92ead into symfony:6.1 Jan 19, 2022
OskarStark added a commit to symfony/symfony-docs that referenced this pull request Jan 19, 2022
@OskarStark
feature #16088 [Notifier] [Bridge] [KazInfoTeh] added bridge document…
6c6184a 
…ation (taranovegor)

This PR was merged into the 6.1 branch.

Discussion
----------

[Notifier] [Bridge] [KazInfoTeh] added bridge documentation

Add Notifier bridge for kazinfoteh.kz

symfony/symfony#44360

Commits
-------

0171f92 [Notifier] [Bridge] [KazInfoTeh] added bridge documentation
@fabpot fabpot mentioned this pull request Apr 15, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot requested changes

@tgalopin tgalopin tgalopin left review comments

@xabbuh xabbuh xabbuh left review comments

@welcoMattic welcoMattic welcoMattic approved these changes

@OskarStark OskarStark OskarStark approved these changes

@chalasr chalasr Awaiting requested review from chalasr

@dunglas dunglas Awaiting requested review from dunglas

@jderusse jderusse Awaiting requested review from jderusse

@lyrixx lyrixx Awaiting requested review from lyrixx

@wouterj wouterj Awaiting requested review from wouterj

@yceruto yceruto Awaiting requested review from yceruto

@stof stof Awaiting requested review from stof

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

Successfully merging this pull request may close these issues.

None yet
8 participants
@taranovegor @OskarStark @fabpot @stof @welcoMattic @tgalopin @xabbuh @carsonbot