★ wanayoo — archive 1999 https://github.com/slimphp/Slim/pull/1975

Nouvelle recherche | Portail wanayoo

slimphp / Slim

Sponsor

Sponsor slimphp/Slim
Watch 546
Star 10.9k
Fork 1.9k

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

Jump to bottom

Ensure withJson returns a new Response #1975

Closed

robertprice wants to merge 2 commits into slimphp:3.x from robertprice:1818

Contributor

robertprice commented Sep 1, 2016 •

edited by akrabat

Fixes issue #1818. The withJson method now clones the Response, and
updates this with a new header, and a new Body.


          Ensure withJson returns a new Response

768306f

Fixes issue 1818. The withJson method now clones the Response, and
updates this with a new header, and a new Body.

coveralls commented Sep 1, 2016

Coverage remained the same at 97.18% when pulling 768306f on robertprice:1818 into 1dedd25 on slimphp:3.x.

mathmarques reviewed

View changes

Slim/Http/Response.php Outdated

  
                $body->rewind();

                $body->write($json = json_encode($data, $encodingOptions));

                $clone = clone $this;

                $clone->body = new Body(fopen('php://temp', 'w+'));

mathmarques Sep 2, 2016
Contributor

I think it's better to use $newResponse = $this->withBody(new Body(fopen('php://temp', 'r+'))) instead of clone and set.

akrabat Sep 2, 2016
Member

Where does newResponse come from?

I like clone as it's quick.

mathmarques Sep 2, 2016 •

edited

Contributor

I was just saying to use withBody, cause if we have a method for return a new response with a new body, we don't need to reimplement it.

robertprice Sep 2, 2016 •

edited

Author Contributor

I think @mathmarques makes a good point. withBody clones and sets the body, so it's the same code really. It may make more sense to do this instead of duplicating code.


          change withJson to use withBody to create a new Response

404321f

Changed withJson to use withBody to create a new Response instead of
cloning and setting a new body directly.

coveralls commented Sep 2, 2016

Coverage decreased (-0.002%) to 97.178% when pulling 404321f on robertprice:1818 into 1dedd25 on slimphp:3.x.

Contributor Author

robertprice commented Sep 2, 2016

Merging #1978 will fix the php7 failing tests.

akrabat closed this in

0899bcb

Member

akrabat commented Oct 11, 2016

Thanks!

akrabat added this to the 3.6.0 milestone

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment