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

Major overhaul for a possible 4.x release, twilio/sdk 8.x #148

Merged
merged 30 commits into from
Nov 26, 2024

Conversation

onlime
Copy link
Collaborator

@onlime onlime commented Nov 18, 2024

As this project seems quite abandoned, I decided to go the opinionated way and updated the following (see CHANGELOG):

  • Update PhpUnit to 10.5 and fixed all tests.
  • Bump twilio/sdk to 8.3
  • Improved types and use constructor property promotion everywhere.
  • Added Pint and fixed PHP syntax.
  • Drop support for PHP < 8.2 BREAKING CHANGE
  • Drop support for Laravel 7.x, 8.x, 9.x, and 10.x BREAKING CHANGE
  • Enable overriding the Twilio message source Enable overriding the Twilio message source #142
  • Add enabled config option (TWILIO_ENABLED) to disable the channel Add enabled config #121

To the maintainers: Thanks for the great work! Please keep this package in shape and try to integrate at least some of the above, updating twilio/sdk to 8.x (see #145).

If the package no longer gets any love here, I will publish it under a different (shorter) name onlime/laravel-twilio and try to do my best maintaining it. I am not an experienced package maintainer, and I am only using Twilio sparingly in my projects, just for simple SMS notifications. Also, I would only support Laravel 11+.

@onlime onlime mentioned this pull request Nov 18, 2024
@atymic
Copy link
Member

atymic commented Nov 23, 2024

Happy to merge this, will review in the next few days. If you'd like I can add you as a maintainer of the package also so you can manage?

@onlime
Copy link
Collaborator Author

onlime commented Nov 23, 2024

Happy to merge this, will review in the next few days. If you'd like I can add you as a maintainer of the package also so you can manage?

Oh, very glad to hear from you. Yes, please do so. I have already published my own fork under onlime/laravel-twilio but I am more than happy to ditch it again in favor of the "official" package, if it's maintained again.

Please add me as maintainer. If we can get this back in shape and I don't need to support legacy Laravel/PHP versions, I'd love to continue maintaining it.

@atymic
Copy link
Member

atymic commented Nov 23, 2024

@onlime you have beed added as a maintainer. Feel free to drop support for all old version, no issues.

@onlime
Copy link
Collaborator Author

onlime commented Nov 23, 2024

@onlime you have beed added as a maintainer. Feel free to drop support for all old version, no issues.

Great, thanks a lot. I have access to the repo now and I have reverted my rebranding in f90ab62, so this PR is ready to be reviewed by you, ready to get merged! 🙏

@jspotten
Copy link

Would love to see this get merged!

@onlime
Copy link
Collaborator Author

onlime commented Nov 26, 2024

Would love to see this get merged!

yes, give @atymic some more time to review it. if not done by the very end of the week, I'm going to merge it.

Copy link
Member

@atymic atymic left a comment

Choose a reason for hiding this comment

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

Good to go. You can approve and merge further changes without me :)

@onlime onlime merged commit ec8d4b1 into laravel-notification-channels:master Nov 26, 2024
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.

6 participants