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.

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

added SMSApi driver #1184

Merged
merged 2 commits into from
Sep 15, 2023
Merged

added SMSApi driver #1184

merged 2 commits into from
Sep 15, 2023

Conversation

kopitar
Copy link
Contributor

@kopitar kopitar commented Sep 7, 2023

Driver contains an additional method Tzsk\Sms\Drivers\SmsApi::with to pass in additional parameters or override existing ones.
On what place in the README.md should I document this functionality?

@tzsk tzsk merged commit d4a75d1 into tzsk:master Sep 15, 2023
4 of 5 checks passed
@tzsk
Copy link
Owner

tzsk commented Sep 15, 2023

Thanks for your contribution 👍

@kopitar
Copy link
Contributor Author

kopitar commented Sep 16, 2023

Driver contains an additional method Tzsk\Sms\Drivers\SmsApi::with to pass in additional parameters or override existing ones. On what place in the README.md should I document this functionality?

@tzsk
This is kind of important for this provider and should be documented. It's your call.

@tzsk
Copy link
Owner

tzsk commented Sep 16, 2023

Hi @kopitar with method needs to be documented in the usage section.

Quick Question: Will the provider work without that parameter? Can there be a suitable default? Can it be supplied via the config rather?

@kopitar
Copy link
Contributor Author

kopitar commented Sep 17, 2023

Quick Question: Will the provider work without that parameter? Can there be a suitable default? Can it be supplied via the config rather?

Yes, it will work with the default config. But this provider expects a cc (country code) parameter on each request and if you are limited to the value in the config you can only send messages to a country that is set in the config.

You can also have more than one sender (sname, from) on one account and you need to be able to switch senders outside of that one sender set as default in the config.

There are also around 10 optional parameters that are rarely used but there should be an option to use them if needed (without polluting the config file with this many rarely used settings).

I will write a very short example in the usage section (in a new pull request).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants