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

#78: upgrade to latest twilio sdk #89

Closed
wants to merge 1 commit into from
Closed

#78: upgrade to latest twilio sdk #89

wants to merge 1 commit into from

Conversation

m1stermanager
Copy link

upgrade twilio to latest sdk and update tests expectations to account for strong types that are now required with the upgrade

first ever PR to this repo (or really honestly any particular open source project at all)

let me know what to address.

…count for strong types that are now required with the upgrade
@m1stermanager m1stermanager changed the title #78 #78: upgrade to latest twilio sdk Apr 7, 2020
"illuminate/support": "^5.5 || ^6.0 || ^7.0",
"illuminate/events": "^5.5 || ^6.0 || ^7.0",
"illuminate/queue": "^5.5 || ^6.0 || ^7.0"
"twilio/sdk": "^6.0"
Copy link
Member

Choose a reason for hiding this comment

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

There's a bunch of breaking changes in the version bump - have you looked through and tested the upgraded version?

https://github.com/twilio/twilio-php/blob/master/CHANGES.md#2020-02-19-version-600

Copy link
Author

Choose a reason for hiding this comment

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

I saw the breaking changes and accounted for where they effected the integration tests.

All tests were passing but I don’t have a particular app that needs or uses all of the functionality here (current use case is sms only) and certainly not every permutation of those features.

I just tried to make sure the tests were passing and hoped I didn’t miss anything in contributor.md etc.

It looks like we consume a versioned api from twilio and the sdk alone is what changed, since I wasn’t adding or changing existing functionality I didn’t do much testing beyond the phpunit tests.

What is the recommended way to test every corner in a real world scenario for future reference?

Copy link
Author

Choose a reason for hiding this comment

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

I also sort of assumed this would accompany a major bump since the sdk has breaking changes but I thought we were fortunate to not be effected by it.

@gregoriohc gregoriohc mentioned this pull request Apr 8, 2020
5 tasks
@gregoriohc
Copy link
Member

Closing this on favor of #90

@gregoriohc gregoriohc closed this Apr 8, 2020
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.

3 participants