Skip to content

Created Recipients object rather than it being a stdClass #103

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

Merged
merged 1 commit into from
Jul 16, 2020

Conversation

JohnstonCode
Copy link
Contributor

@JohnstonCode JohnstonCode commented Oct 18, 2019

the recipients property in MessageBird\Objects\Message is currently just a stdClass. This PR creates a Recipients Object in its place to allow better autocomplete in IDEs

@JohnstonCode
Copy link
Contributor Author

@rfeiner can you have a look at this please?

@modprobe
Copy link
Contributor

modprobe commented Jul 7, 2020

Thanks for your contribution, @JohnstonCode! And my apologies that it has not been picked up for so long.

I personally think it would be super nice to add, however it is also a rather major change.

Do you think we could maybe keep a way to preserve the old behaviour so that existing users can gradually migrate instead of having to do one "big bang" change? Maybe add a parameter to the getter, and by default keep the old behaviour in the first step, and then in a next version switch the default to the Recipient object, before finally dropping the array return completely.

wdyt?

@JohnstonCode
Copy link
Contributor Author

Yeah that makes sense. I will update the PR later today.

@JohnstonCode
Copy link
Contributor Author

Looking back through the code i now remember why i did what i did. So the json_decode here turns MessageBird\Objects\Message->recipients not into an array but into a stdClass. So the changes i have proposed are purely to change the stdClass into Recipients all the fields will map the same so there should be no breaking changes unless people are checking that MessageBird\Objects\Message->recipients is a stdClass

@modprobe
Copy link
Contributor

modprobe commented Jul 7, 2020

Ah, you're right! That's on me for missing that, sorry. I will give this a spin later today and, if all goes well, merge it and release a new version.

Thanks again for your contribution!

@JohnstonCode
Copy link
Contributor Author

No its my fault, I should have explained it better when i opened the PR.

Brilliant, thank you.

@JohnstonCode
Copy link
Contributor Author

@modprobe did you manage to give it a try?

@modprobe modprobe merged commit 0b39ffd into messagebird:master Jul 16, 2020
@modprobe
Copy link
Contributor

modprobe commented Jul 16, 2020

@JohnstonCode Merged and released with v1.19.1! Thanks again!

modprobe pushed a commit that referenced this pull request Jul 20, 2020
modprobe pushed a commit that referenced this pull request Jul 20, 2020
* fix: revert "Created Recipients object rather than it being a stdClass (#103)"

This reverts commit 0b39ffd.

* chore: bump version to 1.19.2
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