twilio-ruby
twilio-ruby copied to clipboard
twilio
fix: Adds as_json to response objects that inherit from Domain.
Fixes #555 Fixes #423
Prevents a stack level too deep error when to_json is called on a response object.
I did not add tests because the issue cannot be replicated at the gem level. However if you would like a test to prove it doesn't break anything I would be happy to include it.
There was no other documentation in the Domain class and since to_json and as_json are fairly well known I didn't think it was necessary to add any.
Checklist
- [x] I acknowledge that all my contributions will be made under the project's license
- [x ] I have made a material change to the repo (functionality, testing, spelling, grammar)
- [x ] I have read the Contribution Guidelines and my PR follows them
- [x ] I have titled the PR appropriately
- [x ] I have updated my branch with the main branch
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] I have added the necessary documentation about the functionality in the appropriate .md file
- [ ] I have added inline documentation to the code I modified
If you have questions, please file a support ticket, or create a GitHub Issue in this repository.
I can get a test to pass if I run it by itself but when I run it with the entire suite it fails. I haven't had time to look into why.
On Tue, Jun 15, 2021 at 9:03 PM Elmer Thomas @.***> wrote:
@.**** commented on this pull request.
In lib/twilio-ruby/framework/domain.rb https://github.com/twilio/twilio-ruby/pull/556#discussion_r652272626:
@@ -31,6 +31,10 @@ def request(method, uri, params = {}, data = {}, headers = {}, auth = nil, timeo timeout ) end
def as_json(*)Hi @rhuberdeau https://github.com/rhuberdeau,
Just checking back in. Would you like us to take it from here or is this still WIP? Thanks!
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/twilio/twilio-ruby/pull/556#discussion_r652272626, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABHV6WFJ52FZBC5NNJEU6LTS7Z47ANCNFSM44XMZQVA .
@rhuberdeau,
Do you mind pushing up what you have? Perhaps we can help. Thanks!
@thinkingserious I merged the most recent and added what I had for tests so far. As I was saying earlier, if you run that one spec it passes but if you run the entire suite it fails.
