Skip to content

[RFC] Return Response objects from the API methods - #19

Closed
mbabker wants to merge 3 commits into
2.0-devfrom
Return-Response
Closed

mbabker wants to merge 3 commits into
2.0-devfrom
Return-Response

Conversation

@mbabker

@mbabker mbabker commented Sep 27, 2015

Copy link
Copy Markdown
Contributor

This fails CI right now because I haven't touched the tests at all, but this serves as an RFC/demo of sorts for how I want to see our social packages structured in 2.0.

In the 1.0 code, our response handlers check for an expected response code and if it doesn't exist, we throw an Exception with an error message and the response code, and on a successful response we return the Response object's decoded body. This limits the usefulness of the API data as we strip away the full response data in both success and error conditions.

This restructures the package to always return a Response object. The success scenario will simply return the Response object unaltered, the fail scenario will still check the response body and throw an Exception, but will now throw the UnexpectedResponseException added to the HTTP package which requires the Response object to be attached to it.

I've elected to bump to the HTTP 2.0 package here in order to use our PSR-7 based Response object, which forces a PHP 5.4 bump based on the underlying dependencies.

Thoughts?

@rdeutz

rdeutz commented Sep 29, 2015

Copy link
Copy Markdown

I also did some work on this because I needed pagination for repositories, I will fork the repo and merge my code. Atm I grabbed the package via composer and changed it local.

@rdeutz

rdeutz commented Sep 29, 2015

Copy link
Copy Markdown

so that is pretty basic far from production ready but it is B/C https://github.com/rdeutz/github-api/tree/pagination-poc

@mbabker mbabker closed this Oct 30, 2015
@mbabker
mbabker deleted the Return-Response branch October 30, 2015 18:06
@mbabker
mbabker restored the Return-Response branch November 23, 2015 17:33
@mbabker mbabker mentioned this pull request Nov 23, 2015
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