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

Replace the abandoned guzzle/parser #34

Merged
merged 5 commits into from
Aug 30, 2015

Conversation

WyriHaximus
Copy link
Member

fixes #23
overrules #26

This replaces the old abandoned guzzle/parser with guzzlehttp/psr7

@WyriHaximus WyriHaximus self-assigned this Aug 22, 2015
@WyriHaximus WyriHaximus mentioned this pull request Aug 22, 2015
@WyriHaximus
Copy link
Member Author

@clue @cboden @jsor Any comments/issues/pointers to fix before I merge this preferably next weekend?

@clue
Copy link
Member

clue commented Aug 23, 2015

👍

@@ -3,7 +3,7 @@
namespace React\HttpClient;

use Evenement\EventEmitterTrait;
use Guzzle\Parser\Message\MessageParser;
use GuzzleHttp\Psr7 as g7;
Copy link
Member

Choose a reason for hiding this comment

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

I'm sorry to nitpick but can we come up with another alias? I used gPsr other places. At the rate Guzzle releases major versions this could quickly get mixed up with misunderstanding "Guzzle7".

@cboden
Copy link
Member

cboden commented Aug 23, 2015

I took my own advice :) reactphp/http#37

@WyriHaximus
Copy link
Member Author

@cboden cheers I took your advise and will merge it 👍

$parser = new MessageParser();
$parsed = $parser->parseResponse($data);
$psrResponse = gPsr\parse_response($data);
$headers = array_map(function(&$val) {
Copy link
Member

Choose a reason for hiding this comment

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

& should be removed. My fault on copy/paste fail

Copy link
Member Author

Choose a reason for hiding this comment

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

Doh! Will fix it 👍

@flip111
Copy link

flip111 commented Aug 26, 2015

👍

@WyriHaximus
Copy link
Member Author

If no one objects to this by tomorrow morning I'm merging it and tagging 0.4.5.

@cboden
Copy link
Member

cboden commented Aug 30, 2015

LGTM

cboden added a commit that referenced this pull request Aug 30, 2015
@cboden cboden merged commit fed9c06 into master Aug 30, 2015
@cboden cboden added this to the v0.4.5 milestone Aug 30, 2015
@WyriHaximus
Copy link
Member Author

Or you just merge it 👍

@WyriHaximus WyriHaximus deleted the guzzle-message-parser-replacement branch August 31, 2015 07:25
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.

Guzzle/parser is abandoned
4 participants